pset 4 recover.c code segmentation fault and problems

Question

here's the code i've reached so far. I know it has a lot of mistakes yet and it also gives me a segmentation fault but i'd really like to understand how to think about fixing it. thanks!

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

 typedef uint8_t BYTE;

int main(void)
{
BYTE buffer[512];

FILE* card = fopen("card.raw", "r");
if (card == NULL)
{
    fclose(card);
    printf("couldn't open card.raw\n");
    return 1;
}

char title[8];
FILE* jpeg = NULL;

fseek(card, 0, SEEK_SET);

do
{
   fread(&buffer, sizeof(buffer), 1, card);
}
while ((buffer[0] != 0xff && buffer[1] != 0xd8 && buffer[2] != 0xff && buffer[3] != 0xe0) || (buffer[0] != 0xff && buffer[1] != 0xd8 && buffer[2] != 0xff && buffer[3] != 0xe1));

fseek(card, -512, SEEK_CUR);

while(fread(buffer, 512, 1, card) == 1)
{    
    fread(&buffer, sizeof(buffer), 1, card);

    if ((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe0) || (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe1))
    {   
        if (jpeg != NULL)
        {
            fclose(jpeg);
        }
        int counter = 0;
        sprintf(title, "%.3d.jpg", counter);
        FILE* jpeg = fopen(title, "a");
        if (jpeg == NULL)
        {
            fclose(jpeg);
            printf("couldn't open jpeg file\n");
            return 2;
        }
        fwrite(&buffer, sizeof(buffer), 1, jpeg);
        counter++;
    }

    else
    {
        fwrite(&buffer, sizeof(buffer), 1, jpeg);
    }


}

fclose(card);
return 0; 
}

Answer 1

Think about simplifying it. For starters, you have 3 different fread statements. Why? You have a block of code that freads until you find the first signature. Why not incorporate it into the next block of code?

In the next block of code, you have an fread() in the while loop setup, immediately followed by another fread. You're reading a block, throwing it away, and reading another block which you try to process. In other words, you're processing only half of the data.

Finally, because of the setup, if a block has a signature, you close the current open output file, if any, and open a new one, and then don't do anything with the current data block,

OR

you write the block out to an output file because you didn't find a signature.

You also have this:

    if (jpeg == NULL)
    {
        fclose(jpeg);

If jpeg really is NULL, it's not pointing at anything. You then try to close a nonexistent file, so you get a seg fault. Guaranteed fail. The NULL means the file didn't open in the first place, so just return.

That should give you a lot to think about.

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