Why does the 0.49 jpg file doesn't match correctly?

Question

I'm doint the recover PSET04. I think, I've done everything correct expect of the 0.49 jpg file. It says that it doesn't match correctly. And the last error in Check50 is "can't check until a frown turns upside down", Why is that?. Please, Help me!!. Am I doing something wrong?

CHECK50: https://submit.cs50.io/check50/58a5af36c1bbd0aa5c54b61d32a6eff014128204

Current Code:

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // If the program doesn't have just only one command line argument
    if (argc != 2)
    {
        printf("Usage: ./recover IMAGE\n");
        return 1;
    }

    // Open forensic image
    FILE *forensicfile = fopen(argv[1], "r");
    if (forensicfile == NULL)
    {
        fclose(forensicfile);
        printf("Could not open: %s.\n", argv[1]);
        return 1;
    }

    // Creating a BYTE array buffer to store the data of a jpeg file
    BYTE buffer[512];

    // Char array that stores the string of a new JPG file
    char filename[8];

    // FILE pointer that stores the data of the JPG file
    FILE *img;

    // JPEG FILE NUMBER
    int JPEGNUMBER = -1;

    // Repeat until we reach the end of the memory card
    while (!feof(forensicfile))
    {
        // Reading JPG files in the memory card
        fread(buffer, 1, 512, forensicfile);
        // Finding a start JPEG FILE
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            JPEGNUMBER++;
            // If its the first JPG file
            if (JPEGNUMBER == 0)
            {
                // If we have find the first JPEG file
                sprintf(filename, "%03i.jpg", JPEGNUMBER);
                // Write into the new jpeg file
                img = fopen(filename, "w");
                if (img == NULL)
                {
                    fclose(img);
                    return 1;
                }
                fwrite(buffer, 1, 512, img);
            }
            // If the first JPG file has been founded, let's create another JPEG file
            if (JPEGNUMBER != 0)
            {
                fclose(img);
                sprintf(filename, "%03i.jpg", JPEGNUMBER);
                img = fopen(filename, "w");
                if (img == NULL)
                {
                    fclose(img);
                    return 1;
                }
                fwrite(buffer, 1, 512, img);
            }
        }
        else
        {
            // If a JPEG file has been founded, we have to keep writting in it.
            if (JPEGNUMBER > 0)
            {
                fwrite(buffer, 1, 512, img);
            }
        }
    }
    // Closing the left files
    fclose(img);
    fclose(forensicfile);
}

Answer 1

This is a very common problem. ;-)

Think carefully about how the while loop ends. It checks to see if the EOF flag is set. If it isn't, it loops again. The problem is that the EOF flag isn't set until after an attempt is made to read past the end of the input file. When the last 512 byte data block is read, there's no more data to be read, but there has been no attempt to read past the end of the file. It's only read up to the end, so the EOF flag is not yet set.

The result is that the loop runs again. Now, the code does try to read past the end of the file on the fread statement. Since there's nothing to read, the statement does nothing, except that it now sets the EOF flag. Consequently, buffer still has the block of data from the previous loop. That data gets written out to the output file. Now, the EOF flag is set, so on the next pass, the while loop terminates, but the damage is done.

The result is that the last block from the input file gets written to the last output file twice. check50 looks for this. ;-)

There are two ways to fix this. The quick and dirty way is to check the EOF flag immediately after the fread. The more elegant way is to incorporate the fread into the while statement and check the return code from fread as the loop condition.

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