0

When running my code for recover (attached below) I'm bouncing back and forth between a segmentation fault and another error when running check50. My code runs without issue on card.raw

But when I try to run check50, I get ":( recovers 000.jpg correctly 000.jpg not found :( recovers middle images correctly 001.jpg not found :( recovers 015.jpg correctly 015.jpg not found"

I know the way I tried to break out of the large while loop was weird, but I didn't know how else to do it. We were warned repeatedly about not using while(!EOF) because it always goes for one too many loops. But fread-ing in and then using fseek to back up again is a slow way of going about solving the issue.

I know that the memory that is used to confirm our code in check50 is different than card.raw, but I don't understand why my code wouldn't work on another batch of memory formatted in the same way.

Any help would be greatly appreciated!

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

#define BLOCK 512

typedef uint8_t (BYTE);

int main(int argc, char** argv)
{
if (argc != 2)
{
    fprintf(stderr, "Usage: ./recover image\n");
}

FILE *file = fopen(argv[1], "r");
if (file == NULL)
{   return 1;   }


int counter = 0;

FILE *img = NULL;

BYTE buffer[BLOCK];


while(fread(&buffer,BLOCK,1,file)==1)
{

fseek(file,-BLOCK,SEEK_CUR);

fread(&buffer, BLOCK,1, file);

bool start = 0;

while( buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
        {
            start=1;
            break;
        }

while(start == 1)
{
    if (counter != 0)
        {fclose(img);}
    char filename[8];
    sprintf(filename, "%03i.jpg",counter);

    img = fopen(filename, "w");
    counter++;
    start = 0;
    break;
}

if(counter > 0)
{
    fwrite(&buffer, 1, BLOCK, img);
}

else
{
    fseek(file, BLOCK, SEEK_CUR);
}
}

fprintf(stdout,"You have reached the end of your files!\n");
        return 0;

}
-1

This part of your code is unnecessary:

fseek(file,-BLOCK,SEEK_CUR);

fread(&buffer, BLOCK,1, file);

Having fseek in the while condition already reads 512 bytes and stores it into buffer, so the above code is equivalent to overwriting buffer with the same information.

The use of a while loop here is wrong:

while( buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
        {
            start=1;
            break;
        }

You should be using an if condition. I believe you meant this as a way to go through the file until the first JPEG is found. If that is indeed the case, then you should be doing something like this:

 while (start == 0)
        {
            // if the first jpeg is found
            if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && 
             (buffer[3] & 0xf0) == 0xe0)
                // set flag as non-zero to signal that first jpg is found
                start = 1;
        }

This will automatically break out of the while loop when start is changed to 1.

The rest of your code does not seem well thought out either. I would suggest you use GDB (and even pen and paper) to figure out how each step of your algorithm gets executed.

| improve this answer | |
  • Thanks, I didn't realize that using fread in the while condition was actually reading the values into the buffer I ended up realizing that both of those didn't need to be while loops, and would be better suited as if's that evaluated each time through the loop. I actually ended up starting over given the additional complexity I added in to the initializing steps. Unfortunately, that leaves me back at the same place yet again – PWalton Oct 16 '17 at 21:02
  • Any idea why it can't find the jpegs in the check50 evaluation? I know my code isn't the prettiest, but I've now rewritten it from scratch, and still can't get it to recognize the images in check50 – PWalton Oct 16 '17 at 21:14
  • After going through your code again, I think the problem is the fseek in the last else condition. You don't need that piece of code because if the counter is not more than 0 (the first JPEG has not been found) you can just do nothing with the data in the buffer and move on. Using fseek effectively skips 512 bytes of code that you don't check for the start of a JPEG. Sorry if my explanation is a bit confusing, I'm traveling at the moment and typing this on my phone. I'll try to edit my response or leave a more understandable comment as soon as I have access to a computer. – nmshaikh Oct 17 '17 at 21:25

You must log in to answer this question.

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