I am trying to complete the assignment given by CS50x (Harvard Community Online, edX.org), and while it successfully recovers images, valgrind is telling me that there are memory leaks that happen in specific lines even though I have nothing allocating memory in said lines. Valgrind's results are shown below:

==4237== Memcheck, a memory error detector
==4237== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4237== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==4237== Command: ./recover card.raw
==4237== HEAP SUMMARY:
==4237==     in use at exit: 552 bytes in 1 blocks
==4237==   total heap usage: 30 allocs, 29 frees, 69,720 bytes allocated
==4237== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==4237==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4237==    by 0x5694E49: __fopen_internal (iofopen.c:65)
==4237==    by 0x5694E49: fopen@@GLIBC_2.2.5 (iofopen.c:89)
==4237==    by 0x400815: main (recover.c:37)
==4237== LEAK SUMMARY:
==4237==    definitely lost: 0 bytes in 0 blocks
==4237==    indirectly lost: 0 bytes in 0 blocks
==4237==      possibly lost: 0 bytes in 0 blocks
==4237==    still reachable: 552 bytes in 1 blocks
==4237==         suppressed: 0 bytes in 0 blocks
==4237== For counts of detected and suppressed errors, rerun with: -v
==4237== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

As you can see, the results are confusing since valgrind says that there are no errors, but there are 552 bytes that have leaked. In the heap summary, it says that there are 30 allocations, but only 29 frees, since my code is opening and closing jpg files in a while loop. For context, my code is shown below:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <stdint.h>

// See if first 4 bytes of the 512 block
// is a JPEG header.
bool isJpegHead(int *buffer)
    if ((buffer[0] & 0xf0ffffff) == 0xe0ffd8ff)
        return true;

    return false;

// Main program loop
int main(int argc, char *argv[])
    if (argc > 1)
        // Open the card file.
        FILE *datafile = fopen(argv[1], "r");

        // Number if jpeg images discovered.
        int numFile = 0;

        // Create filename for the jpeg file.
        char filename[8];

        // Set filename initially as "000.jpg"
        sprintf(filename, "%.03i.jpg", numFile);

        // Create jpg file.
        FILE *jpg = fopen(filename, "w");

        // Loop until end of file.
        while (!feof(datafile))

            // Allocate a 512 byte block of memory.
            int buffer[512];
            fread(buffer, 1, sizeof(buffer), datafile);

            // If the current block has a jpeg header...
            if (isJpegHead(buffer))
                // If it is no longer the first jpg close the current jpg file.
                if (numFile > 0)

                // If it is a new file, create a new file.
                sprintf(filename, "%.03i.jpg", numFile);
                jpg = fopen(filename, "w");

                // Write the current 512 byte block
                // into the opened jpg file.
                fwrite(buffer, 1, sizeof(buffer), jpg);

                // Increment the number of files discovered.
                fwrite(buffer, 1, sizeof(buffer), jpg);


        // If we reached the end of the raw data file,
        // but we haven't discovered a new jpg header,
        // then close the buffer jpg file.

        // Close the raw data file.

        return 0;
        printf("Usage: ./recover image\n");
        return 1;

How my code works: Initially, I allocate a file named "000.jpg". Then, while I haven't reached the end of the data file, I read 512 byte blocks and set it to a buffer. Next, I test the first part of the buffer if it is a jpg header, and if it is then see if it is the first file. If it is not the first file, then close the allocated file and set it to a new file and write the current block to that new file. If it is not a header, then keep writing to the allocated jpg file.

If there are no more headers and we have reached end of the file, then close the allocated jpg file and the raw data file image.

Although it does recover jpg images, it only recovers 13, and there is one memory leak. I do not know why this happens. Could it be I am doing the wrong algorithm? It seems like one of those logic errors that are almost impossible to solve.

As for the insufficient recovery of jpgs, the check50 results are shown below:

Results for cs50/problems/2020/x/recover generated by check50 v3.1.2
:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:( recovers 000.jpg correctly
    recovered image does not match
:( recovers middle images correctly
    recovered image does not match
:( recovers 049.jpg correctly
    049.jpg not found

I think the big clue for me is the fact that there are 50 jpg images in the raw data file, which I am only able to recover 13. Why this is is a mystery to me.

I have tried relocating the numFile increment and the fopen statements, none of which actually worked.


  • A little update: I attempted to change fread to fread(buffer, 1, 512, datafile) instead of fread(buffer, 1, sizeof(buffer), datafile) (and also subsequently the fwrite statements) and it read 50 images. However, it did not get the first image nor the last image. Jul 14 '20 at 20:26
  • Another update: I added an if statement in the else (the one after isJpegHead). The if statement is if (numFile > 0) { fwrite(...); } and it loaded all 50 images, but check50 said that the last image is incorrect: :) recover.c exists. :) recover.c compiles. :) handles lack of forensic image :) recovers 000.jpg correctly :) recovers middle images correctly :( recovers 049.jpg correctly ` recovered image does not match` Jul 14 '20 at 20:42

You must log in to answer this question.

Browse other questions tagged .