[PSET4][Recover] Why does FILE* img work but not FILE *img?

Question

A little background first:

I really enjoyed figuring out recover.c. After finishing the successfully once, I wanted to cement the concepts in my head, so I rewrote it from scratch as best I could, and then debugged it. One single hangup was killing my program, and when I finally figured it out, my second iteration of recover.c worked perfectly.

That hangup was this: FILE* img;

FILE* img;
typedef uint8_t BYTE;
BYTE buffer[BLOCK];
int jpgCount = 0;
char filename;

while (fread(&buffer, BLOCK, 1, inptr))
{
    if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
    {
        if (jpgCount > 0)
        {
            fclose(img);
        }

        sprintf(&filename, "%03i.jpg", jpgCount);

        img = fopen(&filename,"w");

        fwrite(&buffer, BLOCK, 1, img);

       jpgCount++;
    }

  else if (jpgCount > 0)
    {
        fwrite(&buffer, BLOCK, 1, img);
    }
}

When I initially wrote the problem I used FILE *img. The program would recognize jpg's and create jpg files, but they would all be corrupt. However, upon changing FILE *img to FILE* img, it worked.

Why was this happening? Maybe this is a simple question but I've read through the FILE manual pages and looked online but I can't seem to find the answer. Any guidance would be immensely appreciated for my understanding.

Thanks Dylan

Answer 1

FILE* img and FILE *img are the same, you must have been observing some other effect.

While I like grouping the * with the type, as the type of img is FILE*, there's also the argument that you might have several variables on one line, like int* a, b;, which is equivalent to int *a, b;. The first one is misleading, as in both cases, a is a pointer, b is not.

Notice that your variable filename is one byte (char is one byte on any system I've seen so far, although that's not guaranteed), but sprintf(&filename, "%03i.jpg", jpgCount); writes at least eight bytes to that memory position (three digits, ".jpg", and the string terminator '\0'). Use for example an array, like

char filename[8];

and then use that array instead of a pointer to char in

sprintf(filename, "%03i.jpg", jpgCount);

I'd consider buffer and filename the right way to pass a pointer (as those work both on arrays and heap-based memory blocks allocated with malloc), but &buffer and &filename are the same for arrays you define on the stack, which you use here.

You could get rid of the first fwrite if you removed the else in front of if (jpgCount > 0), like

    while (fread(buffer, BLOCK, 1, inptr))
    {
        if (buffer[0] == 0xff &&
            buffer[1] == 0xd8 &&
            buffer[2] == 0xff &&
            (buffer[3] & 0xf0) == 0xe0)
        {
            if (jpgCount > 0)
            {
                fclose(img);
            }
            sprintf(filename, "%03i.jpg", jpgCount);
            img = fopen(filename, "wb");
            jpgCount++;
        }

        if (jpgCount > 0)
        {
            fwrite(buffer, BLOCK, 1, img);
        }
    }
    if (jpgCount > 0)
    {
        fclose(img);
    }
    fclose(inptr);

Don't forget to close any open image files, you don't show this part. valgrind can be used to find those cases, it would complain on non-closed files.