ok I have been looking into this problem for some time now and cant understand what the problem is, My recover.c is creating only first 11 jpegs and only the first one is correct. Any help would be really appreciated

 * recover.c
 * Computer Science 50
 * Problem Set 4
 * Recovers JPEGs from a forensic image.
#include <stdio.h>
#include <stdint.h>
#include <cs50.h>

int main(int argc, char* argv[])
    // open card file
    FILE* inptr = fopen("card.raw", "r");

    // check for errors
    if (inptr == NULL)
        printf ("could not open card. \n");
        return 1;

    // declaring a block of 8 bits of Type BYTE and having 512 blocks
    typedef uint8_t BYTE;
    BYTE block[512];

    int counter = 0;
    while (true)

        // checking for if end of data on card has reached.
        fread(&block, 512, 1, inptr);

        // checking for headers of JPEGs
        if (block[0] == 0xff && block[1] == 0xd8 && block[2] == 0xff && (block[3] == 0xe0 || block[3] == 0xe1))
            char name[8];
            sprintf(name, "00%i.jpg", counter);
            FILE* picture = fopen(name, "w");
            fwrite(&block , 512, 1, picture);

            // going on to next block
            fread(&block, 512, 1, inptr);

            //loop for keep on reading blocks by blocks after the header
            while (block[0] != 0xff && block[1] != 0xd8 && block[2] != 0xff && (block[3] != 0xe0 || block[3] != 0xe1))
                fwrite(&block, 512, 1, picture);
                fread(&block, 512, 1, inptr);


The file count problem has two causes. First, when you get to the 11th file, it labels as 0010.jpg, which you try to stuff into name[8], which will be too small by 1 character. It triggers a failure in the code. You need to change the way the file names are generated so that you get 010.jpg, not 0010.jpg.

Next, a more significant issue. Because of all of the fread() calls, the code reads past the EOF of the input file. You should consider incorporating the fread into the while loop construct.

Third, look at the following:

while (block[0] != 0xff && block[1] != 0xd8 && block[2] != 0xff 
         && (block[3] != 0xe0 || block[3] != 0xe1))

This code will only check for the first three bytes of the signature, not the fourth byte. (block[3] != 0xe0 || block[3] != 0xe1) is always true no matter what the content of block[3] is, because either both tests are not equal, or if one is, the other one cannot be. Because of this problem, you're detecting false start signatures resulting in invalid files.

Once you fix that problem, when you do detect a valid signature, the structure of your code causes the output file to be closed, but then you read another block before opening a new file and processing data into it. The result is that you skip the recovery of every other file.

Some advice. I've found that the more fread() calls, the more complicated the program and the more opportunity for problems that arise. This program can be done with a single fread() and a single fwrite(). When people get to 3 freads, these are exactly the kinds of issues they run into.

There may be other problems, but this should get you going.

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

  • ok I am going through these problems step by step the way you mentioned, made the following changes to my sprintf char name[9]; sprintf(name, "%03i.jpg", counter); and it did corrected the names but now I am only getting files till 014.jpg and 015.jpg is missing, and about the first while loop, is it like necessary to include a fread in its condition or can it work with the way it is right now if I decrease the numbers of freads and fwrites to only one? – Furrukh Jamal Dec 12 '15 at 6:33

You must log in to answer this question.

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