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)
            sprintf(jpegname, "%03d.jpg", i);
            tempjpgg = fopen(jpegname, "w");
            fwrite( tempstorage, 512, 1, tempjpgg);
        else if ( i != 0)
            fwrite(tempstorage, 512, 1, tempjpgg);
        fread(tempstorage, 512, 1, sdcard);
    if (sdcard != NULL)
    return 0;
  • A float makes no sense to me. An uint32_t however would (like uint32_t check = *((uint32_t*) tempstorage);, just not with scanf). You'd have to write full 512 bytes, not 508, and do not by any means fprintf. You fclose tempjpgg without checking whether one is open. Your for loop might better use !feof(sdcard), not without the ! – Blauelf Jan 2 '17 at 12:06
  • Thanks for the help. I did those changes. I ran it in gdb and the check(now firstint) keeps changing its value but it never achieves any value between that range and hence program ends without generating any jpgs. I have updated the source code. Please take a look. And also can you explain why you asked to avoid using scanf and fprintfs. – Shaw Ankush Jan 2 '17 at 19:02
  • scanf and printf is for interpreting or formatting content, it's for text-based formats, not for binary. If there was a number 67 stored as those two digit characters, you'd use scanf, but if it is stored as ASCII character C (has value 67, 'C' == 67 is true), you'd interpret it as a byte, an 8-bit integer, using functions like fread or fgetc. – Blauelf Jan 3 '17 at 10:11
  • Thank you so much! just one more thing => If I use sscanf to read from the tempstorage's pointee into the first int of type uint32_t then also it would read the first 32 bits and then I could check it against the number 0Xffd8ffe0-ef. What potential error can sscanf incur? – Shaw Ankush Jan 4 '17 at 5:38
  • Sscanf is for parsing text representations of data, not binary! – Blauelf Jan 4 '17 at 7:25

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));


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.

  • your code looks more elegant. Logic wise, is doint *firstint = *tempstorage wrong? – Shaw Ankush Jan 4 '17 at 5:42
  • One could do *firstint = *((uint32_t*) tempstorage), which typecasts the pointer, not the pointee (which would have been just the first byte), but it's easier to just have another pointer to the very same memory block, accessing the space as another value type. – Blauelf Jan 4 '17 at 7:33
  • Thank you for that. I did as you said. It produces no photos. Firstint never comes in range ( condition for jpeg). Should check for 0xff, 0xd8 , 0xff individually. I checked that using GDB This code has been updated. – Shaw Ankush Jan 4 '17 at 13:26

You must log in to answer this question.

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