Pset 3 (Recover): Why are images still glitchy?

Question

I've been having trouble with pset3's Recover exercise. My code successfully compiles and creates 50 jpegs, numbered 000 to 049. However, all images are terribly glitchy. Examples:

I've scoured the other Recover-related questions in here and tried checking for the bugs other users have found:

• Accidentally calling fread() twice in a row;
• Getting images of the same size when running ls -al *.jpg;
• Using fopen() improperly;
• Failing to properly check first 4 bytes of header;
• etc.

I have a feeling that maybe the structure of my program is calling some functions in the wrong order, but I've been looking at this code (and running debug50) for hours and still couldn't figure this out.

Could someone give me a hint of what's wrong? Here's the relevant code (edited after question was answered to show only the buggy part):

    ...

    // Name for new file
    char *filename[8];
    sprintf(*filename, "%03i.jpg", imageCount);
    imageCount++;

    // Open new file
    FILE *imageptr = fopen(*filename, "w");

    // Write header
    fwrite(&input, 1, 512, imageptr);

    // Read further
    fread(&input, 1, 512, cardptr);

    ...

Thanks in advance!

Edit: Thanks to curiouswiki and Blauelf's answers, I got the code working! In earlier versions of the program I'd had problems with filename's scope. I tried to solve that using pointers, but went about it the wrong way. After solving my scope problems with the program's structure, I kept the char *filename[8] declaration thinking it was giving me a string and "hey, this couldn't possibly be the wrong line of code..."

Answer 1

Your issue is this:

// Name for new file
char *filename[8];
sprintf(*filename, "%03i.jpg", imageCount);

Rather than declare a string (a null terminated character array) for your filename, you've declared an array of strings (an array of pointers). Those 8 pointers are uninitialized, which means that they contain garbage values.

When you then sprintf to the memory at the address given in the first pointer (ie, *filename) (which is essentially whatever was in that memory before), it just happens to be in the same 512 block of memory where your buffer has been stored. This then puts "000.jpg\0" into your buffer which then gets written to your new jpg file somewhere in the first block.

This of course, makes your jpg file corrupt, as you've seen.

You can actually see this yourself by looking at the hex values of the first 512 bytes of 000.jpg. Easiest way to do this is to run

 xxd -l512 000.jpg

If you scroll through the bytes, you'll spot 000.jpg

Usually, if you'd done this, your program would crash, but just luckily based on how you've used the memory before, your pointer was pointing into the buffer rather than somewhere that would make it crash.

To fix this, change your filename to be a single char array char filename[8]; instead. (Also, your fopen & sprintf will take just filename as their first arguments.)

Answer 2

char *filename[8];

You are declaring an array of 8 pointers, but no space for any characters. Your sprintfed file name might land anywhere, whatever filename[0] initially points to. Change it to char filename[8]; and sprintf to filename, not *filename.

I think that might already explain what you're seeing.

Your logic is not very robust, for example on an input file without JPEG files, you'd run into an infinite loop. And if a JPEG file started on the last block of 512 bytes, you'd write that block twice.

BTW, there are people who consider this kind of glitches "art", changing more or less random bits to enjoy the colours :)