pset4, recover.c: fails to recover middle, end images with check50

Question

First, I have read the forum's discussion rules, and I have reviewed every post relating to recover.c I can find on reddit, StackExchange, and the other discussion boards. Since none seems to address whatever problem affects my code when run on check50, I'm posting now for help.

This is my code:

https://gist.github.com/bruce-mcgoveran/d487f57a2f6033e735a5f9fef57669bc

This is the data generated by CS50's sandbox:

https://sandbox.cs50.net/checks/1a2ca6ca27404d979d12bd9d3a468fa8

As you will see, my code does not recover the middle and last files correctly. At this point, I have no really good explanations for why not. Perhaps I need to refine how I handle extracting data after the last call to fread(), which I realize may well not return a full buffer's (512 bytes) worth of data. However, I can't see how that relates to my code's failure to identify the middle images in the check50 tests. Lastly, to confirm, my code finds and appears to display correctly the 50 images contained within card.raw.

Thank you in advance for any help you can offer to point me in the right direction. Also, if I need to improve how I present my code for help, please let me know - I'm new to posting.

Answer 1

Wow, so many people missing the problem that is hiding in plain sight.

As some people know, check50 uses a different data set than the one provided. (sneaky way to provide a challenge! ;-) ) When check50 runs that data set, it produces 15 files normally, but this code is producing 16 files. So where is the extra file coming from?

The code is detecting a false signature that is buried inside the middle of a block in an image. Why is it doing this? Look at the following line of code:

while (fread(&buffer, sizeof(BLOCK), 1, in))  

BLOCK is a constant for the number 512, the number of bytes that should be read. The problem is that it is reading sizeof(BLOCK). The size of BLOCK, or any constant is 8 bytes. sizeof(BLOCK) is not the value of block. By reading 8 bytes at a time, it's getting snagged on that hidden false signature.

As for the other observations:

filename[8] = '\0'; This has no effect, and in fact, is entirely unnecessary. The sprintf() call will write the '\0' in the appropriate place as needed.

MARS is almost right about the "w" vs. "a" modes, except if I read it right, he has it backwards. If the output file is opened in append mode, it will simply append to an existing file. If the file doesn't exist, it'll work correctly, by accident. The code will only work if the output file doesn't exist. The output file needs to be opened in write mode.

To be honest, I looked at this for an hour and had to enlist a little help. Thanks, @curiouskiwi. ;-) While I picked up on the extra file, she caught the sizeof(BLOCK) issue.

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

Answer 2

Haven't look deep into your code, but one thing looks quite suspicious. You declare char filename[9]. Let's count: 001.jpg looks like 7 symbols + \0 = 8. Why 9? Then, let's look at your definition filename[8] = '\0'. 001.jpg is *char 001.jpg\0 so filename[7] = \0, not filename[8].

I admit I might be wrong, as there could be some logic behind in your code, which I haven't checked in details. However....