0

I wrote some code for recover that seems to me to be recovering the image correctly (I can see pictures of people for all 50 images), but for some reason, check50 says that 049.jpg does not match the image.

My code:

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

bool check(unsigned char stringer[]);

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

    //remember filename
    char* infile = argv[1];

    // open input file
    FILE* inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", infile);
        return 2;
    }

    int index = 0; //The file number at some point
    bool found = false; //Has a jpeg been found yet?
    char currentName[10];  //The name of the current jpeg - ###.jpg
    FILE* namePtr;
    unsigned char current[513];

    while(true)
    {
        if(feof(inptr))
            break;
        fread(&current, 512, 1, inptr);
        if(check(current))
        {
            if(found)
            {
                fclose(namePtr);
                index++;
                sprintf(currentName, "%03d.jpg", index);
                namePtr = fopen(currentName, "w");
                fwrite(&current, 512, 1, namePtr);
            }
            else
            {
                found = true;
                sprintf(currentName, "%03d.jpg", index);
                namePtr = fopen(currentName, "w");
                fwrite(&current, 512, 1, namePtr);
            }
        }else if(found && !check(current))
        {
            fwrite(&current, 512, 1, namePtr);
        }
    }

    fclose(namePtr);
    return 0;
}

bool check(unsigned char stringer[])
{
    if(stringer[0] == 0xff && stringer[1] == 0xd8 && stringer[2] == 0xff && (stringer[3] >= 0xe0 && stringer[3] <= 0xef))
    {
        return true;
    }
    else
        return false;
}


The output of check50 says:

:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:) recovers 000.jpg correctly
:) recovers middle images correctly
:( recovers 049.jpg correctly
    recovered image does not match
See https://cs50.me/checks/a7a89e2edcf434f988efc83f72796277f9c65257 for more detail.


What I got for 049.jpg:

The 049.jpg I got.

What is wrong with my code? Any help would be appreciated.

Thanks!

2
  • man the image is completely fine. But i see that unsigned char current[513]; . Could it be that it was supposed to be 512.
    – CeNiEi
    Jul 17 '19 at 6:08
  • I thought that I would need 512 characters to store 512 bytes and 1 more to store the \0. Also, if that was the problem, even images 000-048.jpg wouldn't be correct. It has to be some problem that appears only in 049.jpg. Jul 17 '19 at 6:31
4

A couple issues. First, the extra byte in the buffer (513 vs. 512) is not needed. The \0 is only necessary when working with strings and the end of string marker is needed. It works correctly because the code is reading and writing 512 bytes. The buffer could be 1000 bytes or more, but as long as it was at least 512 bytes, it would work correctly. Any extra bytes would be unused and would be a waste of memory.

Now, the problem with file 049.jpg. If you were to look, you'd find that your file is exactly 512 bytes too long. Further, the last two blocks of 512 bytes would be identical.

The problem lies here:

    if(feof(inptr))
        break;
    fread(&current, 512, 1, inptr);

The while loop that processes the data checks for the EOF condition, and then does the next read, followed by the rest of the processing. Here's what you need to understand. You can read right up to and including the last byte in a file and have no bytes left, but it won't set the EOF flag. The code has to actually try to read past the last byte in the file to set the EOF condition. That means that the last 512 byte block will be read and processed, the code will loop back and check EOF, see that it has not been set, and will attempt one more read. THAT read will set the EOF, but it will try to process the latest data block. Also, since no data was actually read, the buffer will continue to hold the same data that it had before, i.e., the last 512 bytes of the input file. So, the remaining code will again be processed and written to the output file. ONLY THEN will the loop come back up to the check for EOF and break out.

The result is that you've written the very last 512 bytes to the end of the output file twice.

Instead of a while(true), you might consider putting the fread inside the while statement. If it doesn't read the appropriate amount of data, then it can exit the loop.

This teaches another lesson. You can append any amount of data to the end of an image file and you won't see it displayed. The header data in the file specifies how much data is supposed to be in the image, and will direct the image display software to only display that data. Any extra data is ignored, like the extra 512 bytes in your output file.

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

You must log in to answer this question.

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