0

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

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.

2
  • Thank you for the help! You're right, the position of the * in FILE* img; does not seem to matter, which is surprising but good to know. I'm not sure why it wasn't working but it is now. Your point about the filename[8] makes a lot of sense. I (mistakenly) thought I was declaring an array of an unspecified size, but I see now how in my code it's just a 1 character variable. As for the fwrite, don't I need both for the first case to run? Otherwise on the first passthrough the first jpg will never be written. Right? – Dylan St. John Mar 9 '17 at 19:53
  • I added code that I expect to work, using only one fwrite. – Blauelf Mar 9 '17 at 22:39

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .