1

I got a question regarding check50 on pset4 - recover. If i run check50 a couple times in a row, without even compiling my code in between I get different results (might be all green smileys or showing errors). Is this due to undefined behavior in my code and/or is check50 using random card images?

I can recover all 50 pictures from the given card image, but still, something seems wrong with my code and I am having a hard time fixing this bug as check50 shows no errors three times in a row but shows an error on the fourth check.

I was struggeling to implement the mask to check the first four bits of the fourth byte, so it might have to do with that, but I can't see something wrong there.

This is the relevant part of my code:

//check number or commandline arguments
...
//open and check file
...
//stop and name variables
int stop = 0;
    int readreturn = 0;
    int picturenumber = 0;
    unsigned char buffer[blocksize];
//read until first .jpg header
    while (buffer[0] != 0xff && buffer[1] != 0xd8 && buffer[2] != 0xff && (buffer[3] & 0xf0) != 0xe0 && stop == 0 )
    {
        readreturn = fread(&buffer[0], blocksize, 1, inptr);
    //if end of file is reached stop and return error
        if(readreturn != 1)
        {
            stop =1;
            fprintf(stderr,"no pictures found");
            return 3;
        }
    }
//open and name first file
    FILE* outptr;
    char picturename[8] ={'\0'};
    int d2 = picturenumber/100;
    int d1 = picturenumber%100/10;
    int d0 = picturenumber%10;
    sprintf(picturename, "%i%i%i.jpg", d2,d1,d0);
    outptr = fopen(picturename, "w");
//write files until next .jpg header and iterate until end of file
while(stop==0 )
{
    fwrite(&buffer[0], blocksize, 1, outptr);
//read and check for end of file
    readreturn = fread(&buffer[0], blocksize, 1, inptr);
    if(readreturn != 1)
        stop =1;
//close if next .jpeg header or end of file is reacher
    if((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0) || stop == 1 )
    {
        fclose(outptr);
        picturenumber++;
        d2 = picturenumber/100;
        d1 = picturenumber%100/10;
        d0 = picturenumber%10;
        sprintf(picturename, "%i%i%i.jpg", d2,d1,d0);
    //only open new file if the end of file is not reached
        if (stop !=1)
            outptr = fopen(picturename, "w");
    }
}

Any help highly appreciated. Cheers

edit: updated version

//check number or commandline arguments
...
//open and check file
...
//initialize variables and pointers
FILE *outptr = NULL;
unsigned char buffer[blocksize] = {'0'};
char picturename[8] = {'\0'};
int picturenumber = 0;
int write = 0;
//read through whole data
while (fread(&buffer, blocksize, 1, inptr) == 1)
{
    //find the first .jpg header and open first file
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
    {
        //open output file on first .jpg header
        if (write == 0)
        {
            sprintf(picturename, "%03i.jpg", picturenumber);
            outptr = fopen(picturename, "w");
        }
        //from second header on close output file and open a new file
        if (write == 1)
        {
            fclose(outptr);
            picturenumber++;
            sprintf(picturename, "%03i.jpg", picturenumber);
            outptr = fopen(picturename, "w");
        }
    }
        //don't write before first header
        if (outptr != NULL)
        {
        write = fwrite(&buffer, blocksize, 1, outptr);
        }
}
fclose(outptr);
fclose(inptr);
1

If you haven't omitted code between declaration and first use of buffer, you enter the first while loop and test the content of buffer before you ever load any content into the array, so it contains arbitrary values at that moment. Stack variables and those malloced on heap are not initialised by default (global variables are, by default to zero).

And the negation of

buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0

is

!(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)

or applying De Morgan's Law

buffer[0] != 0xff || buffer[1] != 0xd8 || buffer[2] != 0xff || (buffer[3] & 0xf0) != 0xe0

I'd not do two separate loops, but one. You could even do the fread in the condition, like while (fread(buffer, blocksize, 1, inptr) == 1). Just don't write anything if you so far had no reason to open an output file.

Maybe valgrind could help you, invoked like valgrind ./recover card.raw. Its memory checker will test for various things, including access to memory without defined value. While it produces false positives and false negatives, most output is pointing to actual problems (or their consequences, sometimes those aren't as easy to trace back).

5
  • Thanks for your helpful response. I just figured out, that I didn't pass any values to buffer before I checked it the first time. Filling buffer with 0's after declaration lets me pass check50 every time. Now I'm trying to wrap my head around that if-condition (it's negation). Am I correct that I was just lucky that before the first valid header there was nothing like 0xff 0xd8 0x00. If my program would encounter such data it would falsely open a file. Also, I need to review stack and heap. – merklefl Aug 6 '18 at 10:18
  • That one with && and != would require all bytes to mismatch, which means a single matching byte would leave that first loop. – Blauelf Aug 6 '18 at 11:10
  • And thanks again, I updated the question with my current version, where i picked up the idea of putting everything in one loop. Works well and looks way smoother to me – merklefl Aug 7 '18 at 4:55
  • You probably shouldn't do the header check twice. Also, ever tried formatting like %03i? (format as integer, at least 3 characters, using 0 for padding) – Blauelf Aug 7 '18 at 5:57
  • Those are some good tips, cheers. Updated once more – merklefl Aug 7 '18 at 7:47
1

Good question, had a similar issue regarding the negation. In the end I was able to remove the negation all together. Thanks @Blauelf for your help.

    //init. pointer, variables and arrays
FILE *outptr = NULL;
int counter = 0;
char name[8] = {0};
unsigned char buffer[512] = {'0'};

//main Loop
while (fread(&buffer[0], 1, 512, inptr) == 512)
{
    //.jpg header ID found
    if (buffer[0] == 0xff &&
    buffer[1] == 0xd8 &&
    buffer[2] == 0xff &&
    (buffer[3] & 0xf0) == 0xe0)
    {
        //first .jpg header is found -> only true for 000.jpg
        if (counter == 0)
        { ... }

        //(i-th) .jpg header found
        if (counter > 0)
        { ... }

        fwrite(...);
        counter++;
    }

    //blocks not starting with header bytes
    else
    {
        //continuation of an already opened .jpg
        if (counter != 0)
        {
            fwrite( ... );
        }
    }

//job done! :)
fclose(outptr);
fclose(inptr);
return 0;
}

You must log in to answer this question.

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