pset4 recover compiles, makes jpgs, but doesn't pass check50

Question

My code passes 000.jpg check but misses on middle and end jpgs.

:( recovers middle files correctly :( recovers last file correctly

I looked at the hex code for the jpegs and it seemed fine, the headers were all correct, except that the middle and end jpegs were all ending in several lines of zeros bytes. The first jpeg ended with a few zeros bytes, but not that many. I don't know if that has anything to do with my problems with this. I assume that the jpegs don't all have a number of bytes that are divisible by 512.

Final bytes of 000.jpg:

d900 0000 0000 0000 0000 0000 0000 0000

All the others ended with several lines of zeros.

EDIT: I have re-written my code to count the bytes in fread, but it appears I didn't use that correctly. I'll also post the code that worked but didn't pass check50 at the end.

Broken code after adding "result" variable:

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

typedef uint8_t byte;

int main(int argc, char* argv[])
{
    // ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: ./recover infile\n");
        return 1;
    }

    // create pointer to data file
    char* infile = argv[1];

    // open input file 
    FILE *inptr = fopen(infile, "r");

    // if can't open file
    if (inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", infile);
        return 2;
    }

    // to name jpg outfiles consecutively
    int filecounter = 0;

    int block = 512;
    // to assign 512 bytes to buffer
    byte *buffer = malloc(block);

    char filename[8];  

    // initialize img as NULL pointer
    FILE *img = NULL;

    // to use for testing iterations
    // int testcounter = 0;

    // if end of file not yet reached
    while (!feof(inptr))
    {
        // to read infile to buffer and determine size
        size_t result = fread(buffer, 512,1, inptr);

        // if at start of jpg
        if (buffer[0] == 0xff &&
            buffer[1] == 0xd8 &&
            buffer[2] == 0xff &&
            (buffer[3] & 0xf0) == 0xe0)
        {
            // to see if jpg already created and to close file so new can be 
            created
            if (img != NULL)
            {
                fclose(img);
            }

            // create new file to hold image
            sprintf(filename, "%03i.jpg", filecounter);

            // open new file to write the data
            img = fopen(filename, "w");

            // write data to file
            fwrite(buffer,sizeof(buffer), 1,img);

            // to increase count for naming next jpg
            filecounter++;
        }
        else if(filecounter > 0) 
        {
            if (result == 512)
            {
                fwrite(buffer,sizeof(buffer),1,img);
            }

            else
            {
                // only fwrite actual size
                fwrite(buffer,result,1,img);
            }
        }
    }
    fclose(img);
    free(buffer);

    return 0;
}

My original code is identical to above except:

 // to read infile to buffer and determine size
    fread(buffer, 512,1, inptr);

    // if at start of jpg
    if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
    {
        // to see if jpg already created and to close file so new can be created
        if (img != NULL)
        {
            fclose(img);
        }

        // create new file to hold image
        sprintf(filename, "%03i.jpg", filecounter);

        // open new file to write the data
        img = fopen(filename, "w");

        // write data to file
        fwrite(buffer,sizeof(buffer), 1,img);

        // to increase count for naming next jpg
        filecounter++;
    }
    else if(filecounter > 0) 
    {
        // to add to opened image
        fwrite(buffer,result,1,img);
    }

Answer 1

The heart of the matter is here: fwrite(buffer,sizeof(buffer), 1,img);. buffer is a pointer, sizeof(a pointer) is 8. [If buffer had been declared byte buffer[512] that would have worked].

Once that is fixed, there is another problem here while (!feof(inptr)). This short from last year's courseware describes the problem with that usage better than I ever could. At around 7:30, Jason explains:

....the purpose of feof is not to check if the next call to a read function will hit the end of the file, but rather to check whether or not the end of the file has already been reached. In the case of this example, reading the last line of our file goes perfectly smoothly, but the program doesn't yet know that we've hit the end of our file. It's not until it does one additional read that it counters the end of the file.

Here's a way to avoid !feof and incorporate your result idea. From man fread:

On success, fread() and fwrite() return the number of items read or written. This number equals the number of bytes transferred only when size is 1.

You know that a jpg starts at the beginning of a block (therefore the cruft at the beginning of the file is X number of 512-byte blocks), and you know that "cameras only write to those cards in units of 512 B". Therefore, every fread(buffer, 512,1, inptr); will return 1 (if 512 bytes read) or 0 (ie EOF reached). Think about how you can use fread as the while condition. (Don't forget to remove the explicit fread).