PSet4 Recover - Last file corrupted and check50 finds no correct JPG file

Question

I've spend many hours on CS50's problem set 4 until now and I also searched for my issue in multiple other threads here - still, I couldn't solve my problems.

So I would be glad for anyone looking at my code and pointing me to a possible solution.

I have two major issues:

(1) Last recovered file corrupted: The last file is corrupt and no .jpg-suffix is added. Which obviously means that the last output is not stored correctly in a JPG-file. However I don't see why, since I check for the EOF-condition and I only write the number of bytes to the outfile which have previously been read by fread(). There must be something I'm still missing.

(2) Check50: No files validated correctly: Although I can view all the images I've found (aside from the corrupt one), check50 doesn't validate 000.jpg nor middle images nor 015.jpg as correct JPG images. Maybe this is connected with my first issue.

Here's my code:

    // Description: "Recover" searches an input file for JPG images, recovers them and stores them into multiple consecutive JPG files.


    // Including header files
    #include <stdbool.h>
    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>



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

    // Check correct usage of recover
    if(argc != 2)
    {
        fprintf(stderr, "Usage: ./recover ImageDataFile.raw \n");
        return 1;
    }

    // Open file with image data to be recovered
    FILE *infile = fopen(argv[1], "r");
    // Check if file could be opened
    if(infile == NULL)
    {
        fprintf(stderr, "Could not open infile\n");
        fclose(infile);
        return 2;
    }

    // Define INITIAL PARAMETERS
    int file_num = 0;           // Define file number parameter to increase JPG file numbers: 000.jpg -> 001.jpg -> ...

    // Define buffer parameter to store the next block of 512 Bytes
    uint8_t buffer[512];

    // Define and open new outfile
    char file[17];
    FILE* outfile = NULL;
    //int num_read;

    // While end of file not yet reached;
    while(1)
    {
        // Read in the next 512 bytes and save how many bytes have been read
        int num_read = fread(buffer, 1, 512, infile);

        // Check if end of file has been reached
        if (feof(infile))
        {
            if(outfile != NULL)
            {
                fclose(outfile);
            }

            fclose(infile);
            return 0;
        }

        // Find start of (next) JPG-file: 0xff 0xd8 0xff 0xeX (X = dnt care (upper case X!))
        if( buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && ((buffer[3] & 0xf0) == 0xe0) )
        {
            // Printf and close previous file, if there is any
            if(outfile != NULL)
            {
                sprintf(file, "%03i.jpg", file_num);    // Remember: "%03i.jpg" prints three places!
                fprintf(stdout, "Found new JPG (No. %03i)!\n", file_num);
                file_num++;
                fclose(outfile);
            }

            // Re-Define and open new outfile
            outfile = fopen(file, "w");
            if(outfile == NULL)
            {
                fprintf(stderr, "Could not open outfile\n");
                fclose(outfile);
                return 2;
            }

            // Write first buffer block of the new JPG file, including JPG header
            if(outfile != NULL)
            {
                fwrite(buffer, 1, num_read, outfile);
            }
        }
        // While in current JPG-file, write blocks of 512 bytes to current ###.jpg image file using buffer
        else
        {
            // Write next buffer block to outfile
            if(outfile != NULL)
            {
                fwrite(buffer, 1, num_read, outfile);
            }
        }
    }

    fclose(infile);

    fclose(outfile);

    // In case of success
    return 0;
}

Thanks for any help in advance!

Answer 1

The first if(outfile != NULL) should apply only to fclose(outfile);, the rest has to happen each time.

While I'd add some if(outfile != NULL) also before your last fclose(outfile), that code cannot be reached as the only way you leave the while loop is by return.