When I try to run this program I get a segmentation fault. I think it may have something to do with how I'm checking for the end of the file, although I'm not sure how to go about finding the specific problem. Any ideas?

    * recover.c
    * Computer Science 50
    * Problem Set 4
    * Recovers JPEGs from a forensic image.

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

     typedef uint8_t BYTE;

   int main(int argc, char* argv[])

BYTE buffer[512];
char title[20];
FILE* img = NULL;
int counter = 0;
int padding;

//Open card file
FILE* file = fopen("card.raw", "r");

if(file != NULL)
    //check for EOF
    while(fread(buffer, 512, 2, file) == 2)

        //Read 512 bytes into a buffer
        fread(buffer, 512, 1, file);

        //Start of new jpg?
        if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] <= 0xef && buffer[3] >= 0xe0)
            //close any current files
            if(img != NULL)

            //set counter for jpeg number

            //pad title with zeros
            if(counter < 10)
                padding = 00; 
                padding = 0;

            //create new file
            sprintf(title, "%i%i.jpg", padding, counter);

            //open file
            img = fopen(title, "w");

            //write info in buffer into file
            fwrite(buffer, 512, 1, img);

        //Already found a jpg?
        else if(img != NULL)
            //write buffer info into current file
            fwrite(buffer, 512, 1, img);


    if(fread(buffer, 512, 2, file) != 2)
        //Close any remaining files

return 0;

There's a few problems with your code, not sure which one is causing the segmentation fault.

1) Why are you using 2 as the third argument of fread? You're supposed to read 512 bytes at a time, not 1024.

2) Every time you call fread, it advances. You shouldn't call it more than once in a row without it having a matching call to fwrite (or without making sure whether or not you need to write)

3) An int initialized to 0 and to 00 will only have one value: 0, so the "padding" system will not work.

4) Take a look at the current specs. The signature for this year's assignment is not exactly the same as for last year's

If i had to guess, my first guess would be that the segmentation fault happens due to the first problem i mentioned here. You're trying to read onto indexes of the array far beyond its size

  • The pset 4 recover video brings up the use of using fread in an if condition, checking that there are always at least two blocks left before proceeding. If there is only one block left, that would indicate the EOF block, in which I want my program to close and quit. If I used fseek just after the if condition to go undo the advancement of the pointer, would that work? Feb 5 '16 at 1:59
  • The video puts if (fread(&buf, sizeof(DOG), 2, inp) == 1) As an example of how to use the return of fread in an if block in a comparison. It doesn't say that you should read two blocks. Read one block at a time and check what fread returns. If it returns 0, it's because it didn't read 1 block. You could use fseek in theory, but it will, at least, double the run time of your program, for no good reason
    – Irene
    Feb 5 '16 at 23:51

You must log in to answer this question.

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