Recover pset 4 last image

Question

I have been working on this for a long time. I used check50 on it and it reports that the last image isn't recovered correctly. Can someone provide insight?

    #include <stdio.h>
    #include <cs50.h>
    #include <stdint.h>
    int main(int argc, char* argv[]){
        int cntr=0;
        if(argc != 2){
            fprintf(stderr, "Usage: ./recover source\n");
            return 1;
        }
        FILE* inptr = fopen(argv[1], "r");
        if (inptr == NULL)
        {
            fprintf(stderr, "Could not open %p.\n", inptr);
            return 2;
        }
        int bytec;
        FILE* img = NULL;
        do{
            uint8_t storage[512];
            bytec = fread(&storage, 1, 512, inptr);
            if(cntr == 0 && storage[0] == 0xff && storage[1] == 0xd8 && storage[2] == 0xff && ((storage[3] & 0xf0) == 0xe0)){
                char files[8];
                sprintf(files, "%03i.jpg", cntr);
                img = fopen(files, "w");
                cntr++;
                fwrite(storage, 1, 512, img);
            }else if(cntr > 0 && storage[0] == 0xff && storage[1] == 0xd8 && storage[2] == 0xff && (storage[3] & 0xf0) == 0xe0){
                fclose(img);
                char files[8];
                sprintf(files, "%03i.jpg", cntr);
                img = fopen(files, "w");
                cntr++;
                fwrite(storage, 1, 512, img);
            }else if(cntr==0){

            }else{
                fwrite(storage, 1, 512, img);
            }
        }while(bytec==512);
        uint8_t storage[512];
            bytec = fread(&storage, 1, 512, inptr);
        for(int f=0; f<512;f++){
            if((char)storage[f]!=EOF){
                fwrite(&storage[f], 1, 1, img);
            }else{
              fwrite(&storage[f], 1, 1, img);
              fclose(img); 
              break;
            }
        }
        printf("%d", cntr);
        return 0;
    }

Answer 1

It's a fairly common error with this pset. The last file is one 512 byte block too long. The logic structure of this program is:

1. Read a block of data
2. process the block of data
3. check for EOF

When fread executes, it returns the number of elements read (the third parameter). It will read right up to the end of the file successfully. It will only show something unexpected when it actually tries to read the end of file. A good analogy is a blind person walking to the end of a diving board. The end won't register until he takes one more step and falls in the pool.

Since the final read doesn't actually read anything, storage[] retains whatever was there from the previous read. It gets processed and written to the output file again. Only after that, the code checks to find that it has found the end of the input file.

The code needs to be altered to check for EOF after the read but before the data is processed.

That takes care of the while loop. Next, there's that whole block of code that follows the while loop. It appears to be an attempt to handle a partial block at the end of the input file. Kudos for thinking of it, but there are issues. Assume for now that the above issues are fixed. If the input file ends on a 512 byte boundary (it does), then there won't be anything left to process. This corner case has to be handled. If a partial block is at the end, then the last fread is going to go past EOF anyways. The partial block was the last read done in the while loop, so the final fread breaks the code.

This should give you a bunch to think about. If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

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