Struggling with recover.c

Question

Its getting compiled hence syntactically correct. But after execution it had just made 1 new jpg file and too corrupted. Could you please help me out here. I have used 508 instead of 512 because I am reading 4 bytes of the tempstorage in each iteration and storing it in float type to later check it against 0xffd8ffe0 - ef. See that I have then written the same ( check's 0's and 1's) into tempjpgg before writing those 508 bytes.

  /**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 *
 * Recovers JPEGs from a forensic image.
 */
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
int main(void)
{   
    FILE* sdcard = fopen("card.raw", "r");
    // to check if the file is opened correctly or not
    if ( sdcard == NULL)
    {
        printf("corrupted sd card");
        return 1;
    }
    // file pointer to point to the temporary jpg
    FILE* tempjpgg = NULL;
    // for the nomenclauture of the jpg photos
    char* jpegname = (char*)malloc(8);
    // to read 512 bytes of data at a time from the sdcard
    char* tempstorage = (char*)malloc(512);
    int i = 0;
    uint32_t* firstint = (uint32_t*)tempstorage;
    for (fread(tempstorage, 512, 1,sdcard); !feof(sdcard) ; )
    {
        if ( *firstint >= (uint32_t)0xffd8ffe0 && *firstint <= (uint32_t)0xffd8ffef)
        {
            if (tempjpgg != NULL)
            {   
                if (tempjpgg != NULL)
                fclose(tempjpgg);
            }
            sprintf(jpegname, "%03d.jpg", i);
            tempjpgg = fopen(jpegname, "w");
            fwrite( tempstorage, 512, 1, tempjpgg);
            i++;
        }
        else if ( i != 0)
        {
            fwrite(tempstorage, 512, 1, tempjpgg);
        }
        fread(tempstorage, 512, 1, sdcard);
    }
    if (sdcard != NULL)
    {
        fclose(sdcard);
    }
    return 0;
}

Answer 1

Please initialize

FILE* tempjpgg = NULL;

(In my experience, some compilers initialize to zero automatically, others don't)

Also, firstint should point to the beginning of your 512 bytes, so instead of

uint32_t* firstint = (uint32_t*)malloc(sizeof(uint32_t));

use

uint32_t* firstint = (uint32_t*)tempstorage;

and completely remove

*firstint = *(tempstorage);

which would just take the first byte and store it as an uint32_t.

Also, you might miss the last block, and at the end you have up to two open files.