I've doing recover.c from pset4. When I ran check50, all images except for the last one. I guess it's probably the problem of feof function. But I don't know how to edit my code to make it works

//Creating a BYTE struct
typedef uint8_t BYTE;

if (argc != 2) //remind the user
    printf("You should execute the program with ONE command-line argument.");
    return 1;

//open memory card
FILE *card = fopen(argv[1], "r");

if (card == NULL)
    printf("Could not open file.\n");
    return 1;

//create a buffer
BYTE buffer[512];

//some variables
bool is_first = true; //if this is the first JPEG file
bool ifjpeg = false;//if already found a JPEG
char *filename = malloc(7);
int jpeg = 0;//how many JPEG files

//declare a file pointer
FILE *img = NULL;

//repeat until end of card:
while (!feof(card))

    //read a block into the buffer
    fread(buffer, 512, 1, card);
    //if it's start of new JPEG:
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        //if it's the first JPEG:
        if (is_first)
            //open up a new JPEG file and start writing
            //change the boolean variables
            is_first = false;
            ifjpeg = true;

            sprintf(filename, "%03i.jpg", jpeg); //get the filename
            img = fopen(filename, "w"); //make a file
            fwrite(buffer, 512, 1, img); //start writing
            //close the file I've been writing and open a new one
            jpeg++; //increment the jpeg counter
            fclose(img);//close the file
            sprintf(filename, "%03i.jpg", jpeg);
            img = fopen(filename, "w");//open a new file
            fwrite(buffer, 512, 1, img);

    }//checking first 4 bytes
    else //else: (not the start of a JPEG)
        //if already found a JPEG:
        if (ifjpeg)
            //keep writing
            fwrite(buffer, 512, 1, img);


}//end of the loop

//close all files
return 0;

Please tell me how to edit my code. Thank you very much!


"Ready, FIRE, aim!"

This is a common problem. Look at what the code is doing. It checks for EOF, then reads in and processes the next data block, writing it to the output file, and then checks again for EOF on the next pass of the loop.

Now, think about what happens with the end of the input file. The code reads in the last 512 bytes in the file. Note that it doesn't set the EOF flag because there was no attempt to read beyond the last byte. It writes the last 512 bytes and loops again. Since the EOF flag isn't set yet, it attempts one more read, which does set the EOF flag. Unfortunately, the processing continues because EOF isn't checked here. Also, since there was no data to read, the buffer will be left with whatever was in it, ie, the last 512 bytes read on the previous pass. This data is now written to the output file (meaning it's been written twice now, and the file is 512 bytes too big.) Finally, the loop returns to the top, EOS is tested and found, and the loop terminates.

The code needs to check for EOF after reading from input and before writing to output.

Hint: Have you considered if it is possible to do the read in the while statement?

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

  • It works, thank you so much!
    – E-Lian
    Dec 25 '20 at 6:43

You must log in to answer this question.

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