Recover is Producing Corrupt JPEGS

Question

Here's my code. I've got print debugging statements in there. The output seems to indicated that everything is going well, but when I open up the files, they are blank. I inspected the JPEGS with xxd and they all start with the magic number... so I don't know what's going wrong. Does anybody have any ideas for me?

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

typedef uint8_t BYTE;
char fn[7];

int main(int argc, char *argv[])
{
    // ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: ./recover infile\n");
    return 1;
}

// remember filenames
char *infile = argv[1];

// open input file 
FILE *inptr = fopen(infile, "r");
if (inptr == NULL)
{
    fprintf(stderr, "Could not open %s.\n", infile);
    return 2;
}

int jpegs = 0;

int found_jpeg = 0;

BYTE buffer[512];

do
{
    fread(buffer, 512, 1, inptr);
    if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
    {
        if (found_jpeg)
        {
            printf("Found a NEW jpeg!\n");
            sprintf(fn, "%03i.jpg", jpegs);
            jpegs ++;
            sprintf(fn, "%03i.jpg", jpegs);
            printf("Filename %s\n", fn);
            FILE *img = fopen(fn, "w");
            fwrite(&buffer, sizeof(BYTE), 512, img);
        } else 
        {
            printf("Found a jpeg for the first time!\n");  
            found_jpeg = 1;
            sprintf(fn, "%03i.jpg", jpegs);
            printf("Filename %s\n", fn);
            FILE *img = fopen(fn, "w");
            fwrite(&buffer, sizeof(BYTE), 512, img);
        }
    }
    else if (found_jpeg)
    {
        printf("Writing to existing JPEG...\n");
        sprintf(fn, "%03i.jpg", jpegs);
        FILE *img = fopen(fn, "w");
        fwrite(&buffer, sizeof(BYTE), 512, img);
        printf("Succesfully wrote to %s\n", fn);
    }
}
while (!feof(inptr));
printf("Total: %d\n", jpegs);

// close infile
fclose(inptr);

// success
return 0;
}

Answer 1

There's a problem in this routine:

else if (found_jpeg)
    {
        printf("Writing to existing JPEG...\n");
        sprintf(fn, "%03i.jpg", jpegs);
        FILE *img = fopen(fn, "w");
        fwrite(&buffer, sizeof(BYTE), 512, img);
        printf("Succesfully wrote to %s\n", fn);
    }

Your intention is to "[write] to existing JPEG....". That's not what it is doing. It is declaring a new file pointer here FILE *img = fopen(fn, "w"); and writing to that. Maybe you did it that way because without the declaration (FILE *img), compiler complains about use of undeclared identifier. That's because img can only be used by the code block in which it is declared (it's local to the block), so either the "NEW jpg" block or the "first time" block. Think about where you declared jpegs and why you declared it there. (And remember, you can always declare a variable and not initialize it, ie set it = something, as in BYTE buffer[512];)

Some other observations:

• This char fn[7]; is underallocated. Why? sprintf will "add" the terminating null-byte to make a proper char*. segfault waiting to happen :)
• The program does a lot of fopening, but not much fcloseing. That is a memory leak.
• This while (!feof(inptr)); is a problem (and probably will cause a check50 failure). It is explained very well in this short, starting at around 7:30.