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

typedef uint8_t BYTE;

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

    //check for error in card reading
    if(card == NULL)
        printf("Card reading problemo");
        return 1;

    BYTE buffer[512];
    int jpgno = 0;

    FILE* img = NULL;

        //read 512 bytes of card into the buffer

        if(jpgno == 0)

                //check to find jpg first time
                if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
                    printf("File Found!");                
            }while(buffer[0] != 0xff && buffer[1] != 0xd8 && buffer[2] != 0xff && (buffer[3] != 0xe0 || buffer[3] != 0xe1));

        //if new jpg is found
        if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))  //!!!!!!!!!!!!!!!!!!!
            //close previous file if not the first time
            if(jpgno != 0){


            char* title = "000.jpg";

            if(jpgno > 9)
            sprintf(title, "%d%d.jpg", 0,jpgno);

            if(jpgno < 10)
            sprintf(title, "%d%d%d.jpg", 0,0,jpgno);

            img = fopen(title, "a");

            //error check for img file
            if(img == NULL)
                printf("Img problemo");
                return 2;


        //write 512 byttes of buffer to jpg


When I tried to debug with gdb, it crashes after the do while loop, it skips the if statement and tries to read which causes it to crash, but I dont know what's wrong.

It tried to fseek back but getting weird results.

2 Answers 2


First, on the first pass through the while loop, you attempt to write to img before any output file has been opened. That's triggering the seg fault.

Once that's fixed, there are more problems. You are doing back to back reads at the top of the program, so you will skip processing the first block. If it had a signature, you'd miss a file. (They should figure out a way to test for this error in check50.)

Your while test doesn't work the way you expect. You can't simply change all of the == operators to !=. If any one of the tests is wrong (i.e., say that any one of the signature bytes shows up), it will pass out of the while loop. Random data could do this, and there are cases in the test data that will do this! The way to take the opposite of a long test string like that is this. Say that the test string is test_string. The way to test for it's absence is !( test_string ).

There may be other problems, but there's already enough to work on here.

You have multiple freads and multiple tests for signature blocks. The trend has been that the more of each of these, the less efficient the code, and the more problems arise in code and in debugging. In writing programs, the simpler the better! This program can be written with one read, one write and one test. I'm not saying it has to be, there are always other correct solutions, but think about simplifying your solution.

Finally, you should spend some quality time with style50 and better learn how to format your code. It would make the code a lot easier to read if you formatted it better, and will probably help you sort it out more easily.

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

  • I think if @Jedi18 fixes the do-while test, it should fix the first problem (attempt to write to img before opening file), right? By the way, I much prefer your method for negating long test strings and will let the computer do the math in the future:)
    – Sam Gerber
    Commented Sep 27, 2015 at 16:39
  • I had a semester of digital logic in college that was a blast, but it taught me that it takes time to take a logical complement and its really easy to make mistakes. The big lesson was to "KISS", keep it simple, stupid! That's a lesson I tell myself on every project ;-)
    – Cliff B
    Commented Sep 27, 2015 at 16:45
  • Thank you! That was very helpful. May the force be with you :)
    – Jedi18
    Commented Sep 28, 2015 at 12:30

When you exit your do-while block, this is what buffer holds:

p/x buffer
$11 = {0xf8, 0xff, 0xff, 0xf, 0xff, 0xff, ...

If you look closely, you'll notice this doesn't match the jpg header:

Specifically, the first four bytes of most JPEGs are either

0xff 0xd8 0xff 0xe0


0xff 0xd8 0xff 0xe1

You'll also notice that oddly, this line never executes:

    printf("File Found!");

I think the issue is with logical negation:

Looking at the condition for your do while loop:

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

Let's break it into pieces for the buffer file that kicked us out of the loop

  • buffer[0] != 0xff when buffer[0] is 0xf8 --> true
  • buffer[1] != 0xd8 when buffer[1] is 0xff --> true
  • buffer[2] != 0xff when buffer[2] is 0xff --> FALSE
  • buffer[3] != 0xe0 when buffer[3] is 0x0f --> true
  • buffer[3] != 0xe1 when buffer[3] is 0x0f --> true

So then let's substitute these into your while condition:

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


true && true && FALSE && (true || true)

That evaluates to FALSE, and your loop doesn't continue. Then, you don't enter your if block below it, which, as @CliffB mentioned causes you to execute an fwrite on img before it's opened.

You need to think about where to use AND and where you use OR....

For a tip, try googling Demorgan's law.

  • Thanks a lot!!!! So that's what was wrong, finally understood it.
    – Jedi18
    Commented Sep 28, 2015 at 12:41

You must log in to answer this question.

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