Pset4 Recover: succesfully recovers first and second image, unsure as to why no other image is recovered (spoiler)

Question

I must admit I'm having particular trouble in this assignment. Maybe it's the amount of new concepts. I think I have the idea down but I must me missing an error regarding the loop. I've tried debugging to no avail (particularly because it's a lot to look out for and every image has to go through a lot of iterations). If you could point me in the right direction as to where I'm failing I believe that could be enough for me to try and figure it out, much appreciated.

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./recover image");
        return 1;
    }
    
    FILE *card = fopen(argv[1], "r");
    
    typedef uint8_t BYTE; // creating a BYTE data type
    
    BYTE buffer[512]; // creating a byte array to read file into
    
    int found = 0;
    char filename[7] = "";
    FILE *img;
    int x;
    
    while (fread(buffer, sizeof(BYTE), 512, card) == 512)
    {
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            if (found == 0)
            {
                sprintf(filename, "%03i.jpg", found);
                img = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, img);
                found ++;
            }
            else
            {
                fclose(img);
                sprintf(filename, "%03i.jpg", found);
                img = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, img);
                found++;
            }
        }
        else
        {
            if (found >= 1)
            {
                fwrite(buffer, sizeof(BYTE), 512, img);
            }
        }
    }
    fclose(img);
}

This is what I think I'm doing, if it helps (it's literaly the pseudocode given in the video): // open memory card // repeat until end of card // read 512 bytes into a buffer // if start of new jpg // if first jpeg // create first jpeg // else // close previous, open new // else // if already found jpeg // continue writing to it // close remaining files

Answer 1

Kudos for figuring out that the short array was the cause of the problem. This happens a lot in this pset.

The reason that it's a problem is simple, once you know it. It's also a good lesson on why it's important to make sure that enough memory is always allocated.

When a string is too short for what is being written, the results are unpredictable. C allows the write to happen, usually without throwing an error. (It has its reasons in history. Originally, the concept was to be able to manipulate text stored sequentially.) If the write is large enough that it breaches the memory space allocated to the program, it'll likely throw an error then, but that's rare.

So, when the 9 chars of the file name are written to the 8 char array (including the end of string marker, or 0x00), then the EOS marker ends up being written to the first physical byte in memory that follows the storage reserved for filename. Commonly, this happens to be the first byte of the buffer, corrupting the signature block of the file before it's written out to the file.

That's what's happening.

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

Answer 2

Okey, so my mistake was giving the string (or char *) filename a size of 7 instead of 8, which didn't take into account the NULL character. I'm still unsure why that specific thing caused this kind of error, if anyone understands let me know.