Why can I recover only two images?

Question

This is my code for pset3, recover

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

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

    // remember filename
    char *card = argv[1];

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

    unsigned char buffer[512];
    int i = 0;
    char filename[7];
    FILE *file = NULL;

    while (fread(buffer, 512, 1, memory_card) != 0)
    {
        // if start of new jpeg
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // if already found jpeg
            if (i > 0)
            {
                fclose(file);

            }
            // if haven't found jpeg / after close file
            sprintf(filename, "%03i.jpg", i);
            file = fopen(filename, "w");
            fwrite(buffer, 512, 1, file);
            i++;
        }
        // if not start of new jpeg
        else
        {
            // if already found jpeg
            if (i > 0)
            {
                fwrite(buffer, 512, 1, file);
            }
        }
    }
    fclose(memory_card);
    fclose(file);
    return 0;
}

I can only recover two images, 000.jpg and 001.jpg. The quality of both images are okay, but for some reason I can't get the other 48. Does anybody know the reason why?

Answer 1

char filename[8];, don't forget space for the the null terminator '\0'. sprintf does not know the size of your buffer, so it will just keep writing, writing the null terminator into whatever follows the array. Here it seems to overwrite the counter variable, so that you are overwriting the 001.jpg over and over. Its final content is the actual 049.jpg.

For visualisation, I added some printf to your code:

            printf("i before: %i\n", i);
            sprintf(filename, "%03i.jpg", i);
            printf("i after: %i\n", i);

You wouldn't i expect to change, right? The output however is

~/workspace/freestyle/ $ ./recover card.raw                                                                                                                                                                                                
i before: 0
i after: 0
i before: 1
i after: 0
i before: 1
i after: 0
i before: 1
...

And I'd place another if (i > 0) in front of the second fclose(file);, just in case you don't open a file at all (meaning no JPEG found in the input).