I'm currently writing recover.c for pset5 last exercise. The program I designed runs ok, outputs the expected amount of JPGs at what I would expect to be reasonable sizes. However the JPGs are broken and I cannot figure out why.

I've run it in GDB and I can't find anything wrong, so I would write here the ideas I'm using for the algorithm and, eventually, update with some code if necessary.

The idea is

- define a BYTE as uint8_t and SECTOR as an array of 512 BYTES
- read file, create a jpg counter and a temp SECTOR in which to put the sector currently being read
- start a while loop with condition EOF hasn't been reached
-- read a SECTOR
-- if first 4 BYTES of sector are those of a JPG
---- create a numbered filename, open it with write permission and then augment the jpg counter in one unit
---- create a do while loop which checks for the condition of JPG header in the temp sector (it's a do while because i want to run it a first time without checking, as I have already checked)
---------- inside this loop write temp sector to output file and read another sector (in this order)
---- close file
---- revert position in file by 512 BYTES in order not to skip a sector, as the first step of the next iteration will be to read again
-- close conditional
- close main while loop

I can't see any flawed logic in this and with GDB I've been able to check the state in which I exit the 2nd while loop and the state in which I enter again and the 4 BYTES are in both cases OK.

I now this isn't the way it's intended to be implemented in Zamyla's walkthrough. That's the way I originally thought of it, when I reach a JPG header, I close current file and open a new one. However I thought it might be worthy to try another approach (the one I have tried to implement).

In any case, if you've read up to here, thank you for your time =)

EDIT: Sorry, I forgot to tell you that, the do while has two output conditions, reaching EOF or reading another header. Any of the two breaks the loop.

As for checking for EOF in the outer loop, I'm using Zamyla's advice on fread returning 1 equals non EOF. However, I don't thinks it's relevant anyway as that should cause problems in the last image, not all of them.

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

Is the condition I'm using to check for headers. With an outside ! and nesting the condition inside "(" ")" in the do while loop (i'm too lazy to deconstruct the logical negation, but I think it's even more clean this way).

If it's of any help, this is the code for the file writing

    // write sector to that buffer
    fwrite(&temp_sector, sector_size, 1, outptr);

    // read next sector (if possible) and check for a new jpg
while ( fread(&temp_sector, sector_size, 1, inptr) == 1 && 
        ! (temp_sector[0] == 0xff && temp_sector[1] == 0xd8 &&
           temp_sector[2] == 0xff && (temp_sector[3] == 0xe0 || 
           temp_sector[3] == 0xe1) ) );

sector_size is a constant defined before main, which is 512*sizeof(BYTE), where BYTE is an unsigned 8 bit integer (uint8_t).

  • What if the EOF was reached inside of the do-while loop? You're never checking for it in that loop? Also, maybe some other details are needed. For example, how are you checking for a JPG header? How are you checking for the EOF in the outer loop?
    – kzidane
    Commented Sep 15, 2014 at 20:59
  • Hi @Kareem, thanks for the answer. I've updated the post with that info.
    – Mauro
    Commented Sep 16, 2014 at 5:08
  • I don't see anything wrong with that assuming the other parts are correct. You may send me your source code file at [email protected]!
    – kzidane
    Commented Sep 16, 2014 at 8:09

1 Answer 1


After looking at your code, it turns out that the issue is caused by not specifying enough space for the name of the file which it turn assigns the JPG files invalid extensions.

You only specified a size of 7 chars for the name of a JPG. Given that a name contains 3 digits, a dot, and the extension "jpg", that's a total of 7 chars, but you forgot about the null character. You should definitely specify space for that!

  • Fuck man! The damn +1 error! Hahahaha. I guess the null character was ending up in the begining of the file, that's why it was reading a wrong JPEG header. Great catch!!! Thanks a lot!!!!!!!
    – Mauro
    Commented Sep 16, 2014 at 16:04

You must log in to answer this question.

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