All 50 images look good but failing on middle images on check50 ":( recovers middle images correctly recovered image does not match"

Question

I thought I was there, I was failing on the last image not matching also, but after looking through similar questions I realised that I was checking for EOF before the read of the raw file and not after. That is now solved but it is still failing on middle images. I've looked around here for ages and all I can find is someone alluding to some kind of "false positive" trap in the raw file. I don't understand what to do though. It does say in the background section:

"Odds are, if you find this pattern of four bytes on media known to store photos (e.g., my memory card), they demarcate the start of a JPEG. To be fair, you might encounter these patterns on some disk purely by chance, so data recovery isn’t an exact science."

Is it something to do with that and I am detecting matching first 4 bytes that are not actually a jpeg? I'm getting really stuck so any hints would be much appreciated.

Have copied code below:

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
    //declare variables
    int jpegcounter = 0;
    char filename[8]; //stores 000.jpg format
    FILE* img = NULL;

// ensure proper usage (can only accept one commant line argument)
    if (argc != 2)
    {
        fprintf(stderr, "Enter 1 argument only\n");
        return 1;
    }

    //give filename to input
    char* card_raw = argv[1];

//open card file
FILE *cardfile = fopen(card_raw, "r");

//ensure that file can be openned for reading
if (cardfile == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", card_raw);
        return 2;
    }


//allocate array for block buffer
unsigned char* block_buffer = malloc(sizeof(unsigned char) * 512);

//read into next block off infile if the end of end of file EOF not yet reached
while(fread(block_buffer, (512 * sizeof(unsigned char)), 1, cardfile) == 1)
{

//start of a new JPEG? - YES
    if (block_buffer[0] == 0xff &&
        block_buffer[1] == 0xd8 &&
        block_buffer[2] == 0xff &&
        (block_buffer[3] & 0xf0) == 0xe0)

    {
        if (jpegcounter == 0)//no previous jpeg
        {
            //send formatted filename to array (filname[8])
            sprintf(filename, "%03i.jpg", jpegcounter);
            img = fopen(filename, "w");

            //write bytes to img file until a new jpeg is detected
            fwrite(block_buffer, (512 * sizeof(unsigned char)), 1, img);
            jpegcounter ++;
        }

        else if (jpegcounter > 0) //not the first jpeg
        {
            //Close last img file
            fclose(img);
            sprintf(filename, "%03i.jpg", jpegcounter);
            img = fopen(filename, "w");
            jpegcounter ++;

            fwrite(block_buffer, (512 * sizeof(unsigned char)), 1, img);
        }

    }

// if NOT start of a new jpeg
    if (block_buffer[0] != 0xff)

    {
        if (jpegcounter == 0) //no previous jpeg header
        {
        //do nothing
        }

        else if (jpegcounter > 0)//write bytes to img file until a new jpeg is detected
        {

        fwrite(block_buffer, (512 * sizeof(unsigned char)), 1, img);
        }

    }


}//end while loop

free(block_buffer);

//close infie
fclose(cardfile);

//close outfie
fclose(img);

//success
return 0;

}//end main

Answer 1

The if (block_buffer[0] != 0xff) means you don't copy blocks where the first byte is 0xff.

You could now use

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

(|| instead of && because of de Morgan's laws)

or...

    else

BTW, if you do the same in both parts of an if/else, you could extract the common parts and place them in the unconditionally executed path. And an if with an empty body is essentially just its else, so it would make sense to invert the logic.