I managed to make my code recover all 50 images, and have passed check50. However, during the development process I have encountered two problems, and still doesn't get why it is an issue.

Here is my code

[code hidden]

First, I still don't understand why it is correct to close the program if it does not read 512 bytes, like the code below

while (fread(buffer, 1, 512, file) != 512)
        return 0;

Second, when I assigned the filename to have 7 bytes, it only spits out two images 000.jpg and 001.jpg. Why is this the case? I know that it should be 8 bytes to make room for \0, but if so, why is it still able to release these two filenames, but not others.

I would greatly appreciate an explanation!


You are abusing while but mean if. Since return ends the loop, those are equivalent here, so it still works as intended.

valgrind won't be happy, as you don't fclose your last output file if you have one open.

The disk image (or here card image) consists of sectors of 512 bytes each. We are meant to deal with full sectors only, though you are free to save the number of bytes read in a variable, and write that number of bytes later. Any number of bytes read less than 512 means we've hit end of file.

Your initial check with feof is meaningless, as it will trigger only after an unsuccessful read operation (in which case you already exited). If you want, you could move the fread call to replace the feof.

The thing is a bit more complicated with your actual code: If you are at the end of the file, you still do the four fgetc, and receive EOF each time. Then, you jump 4 bytes back. You are now 4 bytes before end of file (within a block already read and written), and could re-read those 4 bytes. Don't read the four bytes separately. Declare your buffer as byte buffer[512], with byte being defined in cs50.h as a typedef of unsigned char (somewhat more matching the meaning would be using uint8_t, unsigned integer of 8 bits, but there are no practical differences). Read the 512 bytes, and access the four bytes from there.

About the sprintf... it does not care about your buffer length. It writes the eight bytes, whether they fit or not. This means it might overwrite variables behind your actual variable. So maybe that one happened, you overwrote the first byte of some other variable. But that's somewhat unlikely, as variables are aligned at addresses that are a multiple of their length (a 4-byte int would be aligned at addresses divisible by 4), so the 8th byte would likely be unused anyway (because of this padding, such one-off bug would probably cause trouble only for string lengths that are multiple of 4 or 8, meaning they might slip through easily and trigger only in certain situations). So while behaviour is undefined when writing beyond array bounds (as you don't specify the memory layout), I'm still confused how it would show the described behaviour.

You must log in to answer this question.

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