When I try to run my solution to recover in cs50 problem set 4, I get a segmentation fault. I tried to use check50, and I got a poor response for memory leaks.

:( program is free of memory errors

According to the log, this occurred on line 44. Line 44 being:

output = fopen(name, "w");

Does anyone understand why I might be getting an error such as this one?

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
    // Check for more more or less than 2 arguements
    if (argc != 2)
        printf("Usage: ./recover IMAGE\n");
        return 1;

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

    // Check if file is accessibl
    if (input == NULL)
        printf("Could not open file\n");
        return 2;

    // Create the output file, buffer, jgp counter, and file name tracker
    FILE *output = NULL;
    BYTE buffer[512];
    int jpgCount = 0;
    // 3 for chars, 4 for int, and 1 for nul char
    char name[8];

    // Read until end of file. This while loop stops when fread equals zero, because that is when it has nothing left to find
    while (fread(buffer, sizeof(BYTE), 512, input) != 0)
        // Check if there is a jpg
        // Be wary of this code, when I wrote this, it had many memory leaks
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff)
            //Name the file
            sprintf(name, "%03i.jpg", jpgCount);

            // Open the new file
            output = fopen(name, "w");

        // Write information to the file ONLY if it is not blank, otherwise it gives a segmentation fault
        if (output != NULL)
            fwrite(buffer, sizeof(BYTE), 512, output);

    // Close all files

1 Answer 1


First, I can't explain why you get a seg fault. I was unable to duplicate that. Are you maybe compiling and running a different version of the code?

The valgrind memory leak is easy. First, understand that when valgrind reports a memory leak, it's saying where the memory was allocated, not where the problem lies. (It's rare that the problem lies where the memory is allocated.)

Line 44 is where each new output image file is opened. But where are the current output files closed before opening a new one? Remember, output is a file POINTER. That means that memory is allocated for the pointer and the address of the memory is stored in the pointer. This code loads a new address for a new pointer for every new file but nowhere are any of the output files closed. So, you get a memory leak of the size of a file pointer for every output file opened, except the last one. It gets closed after the while loop.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

  • I fixed the memory leak. All that seemed to be needed was when starting the writing loop, just check if the file is empty, if it is not empty, close the file. As for the valgrind issue, thank you for bringing that information to my attention. I was not aware that it only reported the areas where the memory is.
    – Team
    Commented Feb 20, 2023 at 23:28

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .