0

All images are recovered, and when I open them, they all display correctly (I checked every one of them in the IDE), yet my program is not passing check50. Why? Here's my code:

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

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

    // ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: ./recover image\n");
        return 1;
    }

    // create pointer to specified file
    FILE *file = fopen(argv[1], "r");
    if (file == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", argv[1]);
        return 2;
    }

    unsigned char buffer[512];
    int numOfPics = 0;
    while(1)
    {
        fread(&buffer, 1, 512, file);

        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            char buffer2[10];

            // create a new JPEG and increment the number of pictures read
            sprintf(buffer2, "%03i.jpg", numOfPics);
            numOfPics++;
            FILE *img = fopen(buffer2, "w");

            // write the header to new JPEG, then write the remaining bytes
            fwrite(buffer, 1, 512, img);

            do
            {
                fread(buffer, 1, 512, file);
                fwrite(buffer, 1, 512, img);
                if (feof(file))
                {
                    fclose(file);
                    return 0;
                }
            } while (!(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0));

            fclose(img);
            fseek(file, -512, SEEK_CUR);
        }
    }
    return 0;
    fclose(file);
}

EDIT: here's my updated code

unsigned char buffer[512];
    int numOfPics = 0;

        while(fread(&buffer, 1, 512, file))
        {
            char buffer2[10];
            if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
            {
                // create a new JPEG and increment the number of pictures read
                sprintf(buffer2, "%03i.jpg", numOfPics);
                numOfPics++;
                FILE *img = fopen(buffer2, "w");

                // write the header to new JPEG, then write the remaining bytes
                fwrite(&buffer, 1, 512, img);

                fclose(img);
            }
            else
            {
                if (feof(file))
                {
                    fclose(file);
                    return 0;
                }
                FILE *img = fopen(buffer2, "a");
                fwrite(&buffer, 1, 512, img);
                fclose(img);
            }
        }

EDIT2:

unsigned char buffer[512];
int numOfPics = 0;
FILE *img = NULL;
char buffer2[10];

while (fread(&buffer, 1, 512, file))
{
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
    {
        // create a new JPEG and increment the number of pictures read
        sprintf(buffer2, "%03i.jpg", numOfPics);
        numOfPics++;
        if (img)
        {
            fclose(img);
        }
        img = fopen(buffer2, "a");

        // write the header to new JPEG
        fwrite(&buffer, 1, 512, img);

    }

    // write remaining bytes to JPEG
    else if (numOfPics > 0)
    {
        fwrite(&buffer, 1, 512, img);
    }
}
fclose(img);
fclose(file);
return 0;
1

This is a good example of issues hiding in plain sight. Yes, the images look right. But extraneous data at the end of an image file aren't displayed. Image file handlers will only display the amount of data that the header says are supposed to be in the file. There could be gigabytes appended to the file and you'd never see it! The lesson is to look everywhere for additional data. Look at the file size, look at the physical data in the file, in addition to the displayed image!

The code is reading data from the input file and writing to the output file. Here's the problem - when it hits a data block that has a signature, it writes that signature to the end of the current file and then opens a new file, where it writes the signature again. All of the files are 512 bytes too large.

Side note: while(1) -- This is a forever loop, usually a bad idea. Instead, why not incorporate something into the while condition that will end the loop under certain conditions. Seems like the fread would be a good choice, wouldn't it?

Here's another challenge for you. While the code above can be easily fixed to work, the code can be written with a single fread, a single while loop (no do loops), and no need for fseek. ;-)

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

  • Cliff, thanks for your response. I tinkered with the code, wound up with the above in my edit. This time, the pictures are once again coming out fine, and check50 passes for the first and last image, but not the middle images, saying there is a segmentation fault. Also, each time I run it, some strange file with strange characters in the name is also created, and I can't delete them because it says they don't exist! Additionally, once I run the program once, or check it with check50, I can no longer use check50 again - it tries to prepare it, then says something went wrong. What is going on? – jermriddled Apr 10 '19 at 5:03
  • I have to delete the whole folder, make a new one, and copy my files into it just to run check50 once. – jermriddled Apr 10 '19 at 5:07
  • Don't know exactly, not going to try to test just yet. But please explain something to me - why are you opening and closing the output file with every write? The overhead involved is an ugly waste of computing cycles! Instead, when a signature is found, just close the current file (if open) and open the next one! I bet that cleans up some of your trouble. The funny filenames are a good indicator that this is reated to it. – Cliff B Apr 10 '19 at 5:09
  • As for deleting those files, list out the files to see what's there, then do an ls with a wildcard and a pattern that looks something like the wierd named files. Once you have the right pattern with a wildcard that doesn't delete something you want to keep, then use that pattern with an rm command. – Cliff B Apr 10 '19 at 5:12
  • Just realized where the wierd filenames are coming from. What happens before you get to the first signature? What's the filename that you're using to append to? THink about it. – Cliff B Apr 10 '19 at 5:13

You must log in to answer this question.

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