pset4 recover works correctly but fails check50. (not a duplicate because i want to figure out why MY code does not work)

Question

this is my code:

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

int main(int argc, char *argv[])
{
    // Check usage
    if (argc != 2)
    {
        return 1;
    }

    // Open file
    FILE* file = fopen(argv[1], "r");
    if (!file)
    {
        return 1;
    }

    // Read first three bytes
    // unsigned char data[999999];
    // int size = fread(data, 1, 999999, file);

    int num = -1;

    while(1 == 1){
        unsigned char data[512];
        size_t size = fread(data, 1, 512, file);
        if(size < 512 / 4) break;
        if (data[0] == 0xff && data[1] == 0xd8 && data[2] == 0xff)
        {
            num++;
        }
        if (num > -1)
        {
            char filename[7];
            sprintf(filename, "%03i.jpg", num);
            FILE* jpeg = fopen(filename, "w");
            fwrite(&data, 1, 512, jpeg);
        }
    }

    // Close file
    return 0;
}

and this is what check50 says:

:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:( recovers 000.jpg correctly
    recovered image does not match
:( recovers middle images correctly
    recovered image does not match
:( recovers 049.jpg correctly
    recovered image does not match

My code does work (if the images are supposed to be completely transparent) but check 50 says something is off :(. I really need some help and i don't want to use the code from other question(s) like this one that did slightly better than mine in check50, so please don't mark this as duplicate. Thanks for the help!

Answer 1

No, your code only partially works. It is creating 50 files, but they don't contain valid images. Also, if you run ls -al *.jpg, you'll see that every file is 512 bytes long.

After the first signature is found, the code opens a file and writes 512 bytes to that file. The real problem is that it is reopening the file in 'w' mode. This means that each time a 512 byte block is read, the existing file is deleted a new file is created, and the current 512 bytes is written out - but the previous data is lost because the file was deleted when the latest fopen is executed.

The logic is mostly sound, but the output file should only be opened once and only when a new signature is found. For all the data between signature blocks, the data blocks should simply be written to the already opened file.

Also, the signature check is only checking the first 3 bytes. It needs to check the 4th byte too.

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

Answer 2

The following code cause memory corruptions.

            char filename[7];  // this is too small
            sprintf(filename, "%03i.jpg", num);  // this takes 8 bytes.