0

I'm having an issue with recover from PSET4. It works perfectly well with the local copy and raw data. But on running check50, it is killed by the server whilst attempting to create the first jpg. I can't see where I would get an infinite loop though. Here's a shortened version of my code (edited, it's now the full version):

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

typedef uint8_t  BYTE;
typedef uint32_t DWORD;
typedef int32_t  LONG;

// Define constants
#define SIGNATURE 0xffd8ffe0        // Mask to use to check for the signature start to a jpg file
#define REV_SIGN 0xe0ffd8ff         // fread pipes bytes into the variable in reverse order!

// Function definitions
int filesize(FILE *fp);
char* getFileName(char* name, int num);
bool checkBlock(BYTE* testBlock);

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

    char fileName[8];

    FILE* inptr = NULL;
    FILE* outptr = NULL;

    int jpgNumber = 0;
    int filePos = 0;

    LONG numBlocksLeft = 0;

    BYTE block[512];
    DWORD signatureTest;

    bool firstBlock = false;

    /*
    ** Check if there are two arguements or just one (if no filename passed 
    ** assume the file is in the same directory and is called card.raw)
    */

    switch (argc)
    {
        case 2:
            infile = argv[1];
            break;
        case 1:
            infile = "card.raw";
            break;
        default:
            printf("Usage: ./recover (infile)\n");
            return 1;
    }

    // open input file 
    inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        printf("Could not open %s.\n", infile);
        return 2;
    }

    /*
    **  The next task is to determine where the start of the first jpg is.
    **  This will also define the boundaries of the 512-byte blocks.
    **  Do this my searching byte by byte for the key jpeg signature and then
    **  jumping back to the start of that section.
    **
    **  Found that using fread on four bytes ended up with the bytes in the 
    **  reverse order.  The easiest solution was to test against a reversed
    **  signature (REV_SIGN).
    */
    do
    {
        // Record the file position
        filePos = ftell(inptr);

        // Read the first four bytes and compare for the signature
        // fread(&signatureTest, sizeof(DWORD), 1, inptr);

        if (fread(&signatureTest, sizeof(DWORD), 1, inptr) < 1)
        {
            printf("No jpg found\n");
            return 4;
        }

        if ((signatureTest & REV_SIGN) == REV_SIGN)
        {
            firstBlock = true;
        }

        // If the signature block is found, jump back to just before it and 
        // read the whole block
        if (firstBlock)
        {
            fseek(inptr,filePos,SEEK_SET);
        }
        else
        {
            // otherwise jump to the point just after otherwise we end up in
            // an infinite loop
            fseek(inptr,filePos + 1,SEEK_SET);
        }
    } while (!firstBlock);

    /*
    **  Once the start of the jpgs was known, the rest of the file can be 
    **  searched block by block.  If a block was found to start with a 
    **  jpg signature the old file can be closed and a new one opened.
    **  If a block didn't start with a jpg signature it was simply appended
    **  to the existing jpg file.
    */

    numBlocksLeft = ((filesize(inptr) - filePos) / 512);

    while (numBlocksLeft != 0)
    {
        //read next block from infile
        filePos = ftell(inptr);
        fread(&block, 512, 1, inptr);

        if (checkBlock(block))
        {
            //Close the last jpeg file (skip for the first file!)
            if (jpgNumber != 0)
            {
                fclose(outptr);
            }

            //Open the next jpg
            strcpy(fileName, getFileName(fileName,jpgNumber));

            outptr = fopen(fileName, "w");
            if (outptr == NULL)
            {
                return 3;
            }

            //write the first block into the new file
            fwrite(&block, 512, 1, outptr);

            jpgNumber++;
        }
        else
        {
            // Skip if the first blocks aren't part of a jpeg file
            fwrite(&block, 512, 1, outptr);
        }

        numBlocksLeft--;
    }

    // Tidy up by closing the remaining files
    fclose(outptr);
    fclose(inptr);

    return 0;
}

int filesize(FILE *fp)
{
    // File size function obtained from:
    // http://stackoverflow.com/questions/238603/how-can-i-get-a-files-size-in-c

    int prev = ftell(fp);       // record the initial position
    fseek(fp, 0L, SEEK_END);    // scroll through to the end of the file
    int sz = ftell(fp);         // note the position
    fseek(fp,prev,SEEK_SET);    // go back to where we were
    return sz;
}

/*
** Function to generate the name of a new sequential jpg file
** File name is padded out with leading zeros if required.
*/
char* getFileName(char* name, int num)
{
    char numStr[3];

    //convert the file number into a string
    sprintf(numStr,"%i", num);

    // Pad out leading zeros
    if (num < 100)
    {
        name[0] = '0';
        name[1] = '\0';

        if (num < 10)
        {
            name[1] = '0';
            name[2] = '\0';
        }
    }

    // Finally link the padding, file number and file tag together
    strcat(name, numStr);
    strcat(name, ".jpg\0");

    return name;
}

bool checkBlock(BYTE* testBlock)
{
    // Combine the first four bytes ready to check against the SIGNATURE start of a jpg file
    DWORD startBytes = testBlock[3] | (testBlock[2] << 8) | (testBlock[1] << 16) | (testBlock[0] << 24);

    // Check to see if it the start of a new file
    // the value of startBytes may be different to SIGNATURE only in the 
    // four least significant digits.
    return ((startBytes & SIGNATURE) == SIGNATURE);   
}

I've tested that it returns from an end of file during the search for the first block, and the second search on a block by block basis has only a limited number of iterations.

Editted to show the full code following complaints that I had edited it too far.

7
  • 1
    How are we supposed to debug your code and help you with your problem with many pieces missing? Please see How to create a Minimal, Complete, and Verifiable example and update your question.
    – ChrisG
    Aug 12 '16 at 12:37
  • Could your program get stuck in a loop there is a jpg in card.raw with one of the other 15 signatures defined in the spec? Aug 12 '16 at 13:31
  • While the problem code may or may not be present above, there are just too many pieces missing. I wouldn't attempt to analyze this because it is too likely that the problem is missing from the code or additional false problems will appear because of assumptions that have to be made. Please edit the question and fill in the gaps.
    – Cliff B
    Aug 12 '16 at 16:35
  • My apologies, as it is a marked assessment I was trying to limit views to my full solution. Here's the full code: Aug 12 '16 at 22:34
  • Since the timeout is happening before it decodes the first jpg, I'm assuming my error is in the first search to find the start of the first jpeg. Yet it works fine with the local version, with the 50 jpg raw file. Scans though it in about three seconds when I force it to skip though the whole file, in that case it successfully detects the end of file when fread returns less than 1. Aug 12 '16 at 22:46
0

Two problems with this code, one serious and one that isn't fully tested for. First, while processing the leading garbage data, the code is checking every byte for a signature. It should only be checking the first 4 bytes of each 512 byte block. It is possible for image data to actually be the same as a signature block. If you test every byte for a signature and the signature string exists inside the image data, you'll get a false positive and start in the wrong location. This is actually built into the test data and your code is getting caught by it.

Next, while it isn't rigorously tested by check50, the code only looks for one signature when there are actually 16 different signatures to be checked for. You're just getting lucky that this isn't tripping you up.

Both of these items are in the program specification. This is one reason why it is critical to write exactly to the code spec, not close, no more and no less.

And yes, the test data used by check50 for this pset is different from what is provided. Consider it a lesson in writing complete and thorough test data for a project. ;-)

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

2
  • The bitwise AND with 0xe0 allows the other scenarios (0xe1 to 0xef to be accepted, unfortunately I've just seen that my test is really flawed, as it will also accept 0xf0 to 0xff, similarly byte 2 will also accept other values! What I need to do is AND with 0xfffffff0 and then OR with SIGNATURE. I will have a look at that later when I have time to sit down and look at it. Aug 13 '16 at 9:47
  • Thanks, it was the signature within data in the first non-image sectors that was messing with it and my bit-wise search for the start of the first image. I just made that first search an option using an extra optional flag. Sep 2 '16 at 9:48

You must log in to answer this question.

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