0

My code is able to generate images from card.raw dataset, but fails with the check50 dataset, giving the following error:

:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:( recovers 000.jpg correctly
    timed out while waiting for program to exit
:( recovers middle images correctly
    timed out while waiting for program to exit
:( recovers 015.jpg correctly
    timed out while waiting for program to exit

I looked through this forum for possible solutions and tried to tie up any loose ends like closing the files etc. The time taken to generate the 50 images from card.raw is well within a second.

I have been at this for quite some time and I am not able to see a way out! Any help is greatly appreciated!

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

// function that checks for jpg header at the start of a block
int find_jpg(unsigned char array[]);

int main(int argc, char *argv[])
{
    // check for number of arguments
    if (argc !=2)
    {
        fprintf(stderr, "Correct usage: ./recover filename\n");
        return 1;
    }

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

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

    // check if infile is valid
    if (inptr == NULL)
    {
        fprintf(stderr, "Invalid file\n");
        return 2;
    }

    // declaring few variables
    bool jpg_found = false;
    int jpg_size = 512;
    unsigned char buffer[jpg_size];

    char out_jpg[7];
    int file_counter = 0;

    // run loop till jpg header is found
    while (!jpg_found)
    {
        fread(buffer, jpg_size, 1, inptr);

        // enter if jpg header is found
        if (find_jpg(buffer))
        {
            jpg_found = true;

            // open new out file
            sprintf(out_jpg, "%03i.jpg", file_counter);
            FILE *outptr = fopen(out_jpg, "w");
            fwrite(buffer, jpg_size, 1, outptr);

            // check for invalid out file
            if (outptr == NULL)
            {
                fprintf(stderr, "Invalid file\n");
                return 2;
            }

            // check for end of file
            while (fread(buffer,jpg_size,1,inptr) > 0)
            {
                // enter if new jpg header is found and open new out file
                if (find_jpg(buffer))
                {
                    fclose(outptr);
                    file_counter = file_counter + 1;
                    sprintf(out_jpg, "%03i.jpg", file_counter);
                    outptr = fopen(out_jpg, "w");

                    // check for invalid out file
                    if (outptr == NULL)
                    {
                        fprintf(stderr, "Invalid file\n");
                        return 2;
                    }
                }
                fwrite(buffer, jpg_size, 1, outptr);
            }
            fclose(outptr);
            break;
        }

        // to read the next 512 byte block for the jpg header
        fseek(inptr, -(jpg_size - 1), SEEK_CUR);
    }
    fclose(inptr);
}

// function that checks for jpg header at the start of a block
int find_jpg(unsigned char array[])
{
    if (array[0] == 0xff &&
        array[1] == 0xd8 &&
        array[2] == 0xff &&
        (array[3] & 0xff) == 0xe0)
        return 1;
    else
        return 0;
}
0

There are three issues with the code. First, char out_jpg[7]; doesn't allow room for the end of string marker, \0. Because of this, anytime the code loads the filename into out_jpg, it will overwrite the first byte of whatever is physically next in memory.

Second, check50 uses a different test data file to check the program. One of the differences is that it has 3 files with the following signature: ff d8 ff e1. These three files are not being found. Take a close look at the find_jpg() function. It's an easy fix once you find the problem.

Finally, the big issue. While you think it's running fast enough, check50 expects it to run a lot faster. It's timing out on all the checks, even when the first two issues are fixed. While the code is technically working correctly after those first two fixes, it will need to be rewritten to correct the problem.

Here's some advice on that. Anytime a block of code appears identically or nearly identically in code, especially in nested loops, that should be a red flag that there's probably a more efficient or simpler way to write the code.

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

2
  • Thanks very much for the help! I corrected the first two issues which, as you said, were simple enough. Then, I rewrote the code in a simpler and straightforward manner, but it was still timing out on check50. Then I removed the fseek operation from the code and suddenly, it passes the check50 test. Although I am now over the moon that it worked, isn't the fseek operation necessary to check for the jpeg signature that can begin anywhere on the memory instead of just checking for it at the beginning of each 512 byte block on the memory? – Sashi Dec 27 '17 at 16:18
  • That explains it. The signature MUST be at the beginning of each block and not in the middle of a block. Two things to remember. 1. Files start on a block boundary, not in the middle of a block. 2. An image file could theoretically contain data that coincidentally match a signature. Checking every byte sequence could produce a false positive. In fact, there is a false positive signature planted inside the test data used by check50. I'm surprised that you didn't bump into that. ;-) – Cliff B Dec 28 '17 at 2:54

You must log in to answer this question.

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