Problem Set 4: Recover

Evidence of a 5-point Submission

Does not use any magic numbers (uses variables for the first three hex bytes when checking the header)
Uses a bitwise operator (likely the & operator) to check the first four bits of the fourth header byte
When opening a new file, uses an if statement to check if the file pointer being written to is NULL—closes the file if so
Only uses fwrite once-an-iteration within the jpeg discovery/creation loop (minimal repeated code between the case where header is found versus not found)
Uses the ++ operator inside of the sprintf call to increment the file count
Error handles within the while loop to ensure that fopen and fwrite calls work properly

Evidence of a 4-point Submission

Does not use any magic numbers (may use hex numbers when checking the header)
Uses some operation (likely ÷ 0x10) to check the first four bits of the fourth header byte
Uses an if statement to check if a file is currently open and needs to be closed (may check if the number of files recovered is not zero)
Only uses fwrite once within the while loop (minimal repeated code between the case where header is found versus not found)

Evidence of a 3-point Submission

Reads in blocks of size BLOCK_SIZE and writes to a new file when a header is found
Uses comparison operators to check the fourth header byte against a range of values
May separately handle cases when a header is found or not found in separate conditional statements (repeats fwrite code twice)
Uses a counter variable to keep track of the file number
Closes the current file when a new header is found
Closes all open files after the while loop terminates

Evidence of a 2-point Submission

Does not check if the file has been opened properly
Uses the || operator to check for all 16 possible options for the fourth header byte
Uses a condition in the while loop condition that does not include an fread function call
Uses a condition in the while loop condition that compares the return of an fread function call to a number other than BLOCK_SIZE
Uses unnecessary or confusing variables within the while loop
Does not properly close all files when then while loop terminates
Uses malloc (possibly for a string for the filename) but then does not free all of the memory at the end of the while loop

Evidence of a 1-point Submission

Opens the file in a mode other than "r"
Checks for the fourth header byte using a method that is confusion or completely unnecessary
Uses a looping mechanism other than the while loop to iterate through the file
Does not properly close all files when the while loop terminates

Example Implementations (Worse vs. Better)

Checking Command Line Arguments

Worse Implementation

The example below uses two unnecessary conditionals (just != would suffice), and unnecessarily wraps the rest of the code in an else statement

if (argc > 2 || argc < 2)
{
    printf("Correct Usage: ./recover card.raw");
    return 1;
}
else
{
    // the rest of the program...

Better Implementation

The example below uses a single if statement to check the number of arguments

if (argc != 2)
{
    printf("Correct Usage: ./recover card.raw");
    return 1;
}
// the rest of the program...

Loop Implementation

Worse Implementation

This example uses an unnecessary variable to determine if the EOF has been reached

bool still_reading = true;
while (still_reading)
{
    // writing to files within this loop...

    if (fread(buffer, 1, BLOCK_SIZE, raw_file) != BLOCK_SIZE)
    {
        still_reading = false;
    }
}

Better Implementation

This example makes use of the return of the fread function to eliminate the need for an extra variable

while (fread(buffer, 1, BLOCK_SIZE, raw_file) == BLOCK_SIZE)
{
    // writing to files within this loop...
}

Checking for a Header

Worse Implementation

This example is hard to read and requires two extra comparisons to check the fourth header byte

if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] >= 0xe0 && buffer[3] <= 0xef))
{
    // header handling here...
}

Better Implementation

The example below uses globally defined variables for the first three header bytes, and uses a bitwise operator to check the fourth header byte. Additionally, avoids long line length

#define SOI_0 0xff
#define SOI_1 0xd8
#define APPN  0xff

// ... rest of the function

if (header[0] == SOI_0 &&
    header[1] == SOI_1 && 
    header[2] == APPN &&
    ((header[3] & 0xf0) == 0xe0)

Handling Files

Worse Implementation

This example includes unnecessary conditionals (resulting in unnecessarily repeated code)

if (/* checks for header here */)
{
    if (jpg_number != 0)
    {
        fclose(img);
        sprintf(filename, "%03i.jpg", jpg_number);
        img = fopen(filename, "w");
        fwrite(buffer, 1, BLOCK_SIZE, img);
        jpg_number++;
    }
    else
    {
        sprintf(filename, "%03i.jpg", jpg_number);
        img = fopen(filename, "w");
        fwrite(buffer, 1, BLOCK_SIZE, img);
        jpg_number++;
    }
}
else
{
    fwrite(buffer, 1, BLOCK_SIZE, img);
}

Better Implementation

This example minimizes repeated code, and also ensures the fopen and fwrite functions succeed. Additionally, this method uses a globally defined variable to define the filename format, and uses the ++ operator within the sprintf function to increment the counter in the same line

#define JPEG_FILENAME_FORMAT "%03i.jpg"

// rest of the function

if (/* checks for header here */)
{
    if (jpeg_file)
    {
        fclose(jpeg_file);
    }

    sprintf(jpeg_filename, JPEG_FILENAME_FORMAT, num_jpegs_recovered++);

    jpeg_file = fopen(jpeg_filename, "w+");
    if (jpeg_file == NULL)
    {
        fclose(raw_file);
        printf("recover: %s: Error creating file\n", jpeg_filename);
        return 1;
    }
}

if (jpeg_file)
{
    if (fwrite(buffer, 1, BLOCK_SIZE, jpeg_file) != BLOCK_SIZE)
    {
        fclose(jpeg_file);
        fclose(raw_file);
        printf("recover: %s: Error writing file\n", jpeg_filename);
        return 1;
    }
}