Valgrind detects reachable blocks even after freeing malloc space in C (cs50 Problem Set 4 Recover)

I have completed the Problem Set 4 of cs50 called recover. You will see below that i have used malloc for a char* called sfile. In the end of the file i remember to free the allocated space (aka sfile) and close the 2 documents that i have opened.

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

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./recover IMAGE\n");
        return 1;
    }
    printf("%s", argv[1]);

    FILE *file = fopen(argv[1], "r");

    if (file == NULL)
    {
        printf(" can't be opened for reading\n");
        return 2;
    }

    unsigned char arr[512];

    int s = 0;

    FILE *output_file = NULL;

    char *sfile = malloc(sizeof(char) * 8);

    while (fread(arr, sizeof(char), 512, file) != 0)
    {

        if (arr[0] == 0xff && arr[1] == 0xd8 && arr[2] == 0xff && (arr[3] & 0xf0) == 0xe0)
        {
            sprintf(sfile, "%03i.jpg", s);

            output_file = fopen(sfile, "w");

            s++;
        }

        if (output_file != NULL)
        {
            fwrite(arr, sizeof(char), 512, output_file);
        }

    }

    free(sfile);

    fclose(output_file);
    fclose(file);


    return 0;
}

However when running check50 which is a tool to check if you completed all the points of the exercise, i got only one red message indicating that valgrind has detected memory errors. So my next step was to run:

$ valgrind ./recover card.raw

this is what returned:

==53590== HEAP SUMMARY:
==53590==     in use at exit: 23,128 bytes in 49 blocks
==53590==   total heap usage: 1,091 allocs, 1,042 frees, 240,880 bytes allocated
==53590== 
==53590== 23,128 bytes in 49 blocks are still reachable in loss record 1 of 1
==53590==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==53590==    by 0x4A076CD: __fopen_internal (iofopen.c:65)
==53590==    by 0x4A076CD: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==53590==    by 0x109304: main (recover.c:37)
==53590== 
==53590== LEAK SUMMARY:
==53590==    definitely lost: 0 bytes in 0 blocks
==53590==    indirectly lost: 0 bytes in 0 blocks
==53590==      possibly lost: 0 bytes in 0 blocks
==53590==    still reachable: 23,128 bytes in 49 blocks
==53590==         suppressed: 0 bytes in 0 blocks
==53590== 
==53590== For lists of detected and suppressed errors, rerun with: -s
==53590== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The program works perfectly and this is the only problem there is left.

I have looked at cs50 recover sill reachable but no valgrind error, but I could not really understand it since his code is spaghetti code to my eyes (No offence!!)

I am still new to C and having a programming background in Python makes it difficult for me to understand anything that has to do with memory, since python is much more straight forward.

Any help is appreciated 🙂

>Solution :

The problem is that

output_file = fopen(sfile, "w");

will happen multiple times. But you only close output_file once.

You need to add a check to see if output_file is not NULL and then close the file first, before opening it again.

The main hint is that the leak happens in fopen. Which is easy to see in the Valgrind output.