Pset4 recover.c check50 error: timed out while waiting for program to exit

Question

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;
}

Answer 1

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. ;-)