0

I have a problem where I need to check if I have an open file on my loop, so I properly close the previous file before writing a new one. But my check only works if I fclose(img) if my counter > 0. The problem here is that (img) is not actually declared until after the check, but if I put it above the check it renders the check pointless. How do I reformat this code so that I can fclose(img) if my counter > 0, while also declaring img before the counter somehow?

    do
{
        unsigned char read[512];
        fread(&read, 512, 1, memcard);
        if (read[0] == 0xff && read[1] == 0xd8 && read[2] == 0xff && (read[3] & 0xf0) == 0xe0)
        {
            if(counter > 0)
            {
                fclose(img);
            }

            counter++;
            sprintf(filename, "%03i.jpg", counter);
            FILE *img = fopen(filename, "w");

        }
        if (counter > 0)
        {
            fwrite(&read, 512, 1, img);
        }
        readNum = fread(&read, 512, 1, memcard);
}
while (readNum == 512);

Edited Code

int counter = 0; // Initialize counter for .jpgs found
char *filename = NULL;
FILE *img = NULL;
unsigned char read[512];
while (fread(&read, 1, 512, memcard) == 512)
{
        //unsigned char read[512];
        //fread(&read, 1, 512, memcard);
        if (read[0] == 0xff && read[1] == 0xd8 && read[2] == 0xff && (read[3] & 0xf0) == 0xe0)
        {
            if(counter > 0)
            {
                fclose(img);
            }

            counter++;
            sprintf(filename, "%i.jpg", counter);
            img = fopen(filename, "w");

        }
        if (counter > 0)
        {
            fwrite(&read, 1, 512, img);
        }

}
1
  • 1
    Could I just declare FILE*img = NULL on a line above, only assigning it a value afterwards?
    – Coal143
    Apr 26 '18 at 15:21
0

That's a scope issue.

If you declare a variable, it exists on stack until you leave this block. You declare img on the last line of the block, hit the }, and - poof - the stack pointer gets reset to the state it had before entering the block, and now img no longer exists (its content still exists, and the next pointer you declare will likely be at the same position on stack and contain that value, but the variable has disappeared). Similar problem with read, but that has more problems, like that you read two sectors per iteration, but write only one.

I'd suggest that you declare both img and your buffer outside of the loop.

And the first number in fread is the block size, the second is the number of blocks to read, the function returns the number of blocks it successfully read. fread(read, 512, 1, memcard) would return 0 or 1, fread(read, 1, 512, memcard) would return something from 0 to 512.

Also, what if your input file is empty? You would enter the loop (as it's a do..while loop), fread would return 0 for no blocks read (but you ignore the value), and you'd test arbitrary data for representing a JPEG header. Behaviour is undefined.

One solution to processing existing sectors only is to fread, then check feof and leave the loop if it returns truthy (feof checks a bit set on an unsuccessful read). Or use fread in the condition of a while loop (not do..while, for reasons above), and immediately check its return value. But whatever you do, try to avoid multiple fread, as you already did for fwrite.

4
  • Why is it important to avoid multiple fread? Or rephrased why is readNum = fread(&read, 1, 512, memcard); bad code? As far as I can tell it is indeed assigning 512 to readNum when all bytes are read, so it seems to be working on that end. What bad can happen?
    – Coal143
    Apr 26 '18 at 16:48
  • As I said, you read two sectors and write one, so you drop the other.
    – Blauelf
    Apr 26 '18 at 16:56
  • Thanks for the help. I think most of this makes sense now, but I'm ending up getting this error: SIGSEGV Segmentation fault whenever sprintf(filename, "%i.jpg", counter); triggers. I've added new the code on the OP
    – Coal143
    Apr 26 '18 at 17:10
  • Ah, I understand. I was pre-creating the string. Couldn't fill a void
    – Coal143
    Apr 26 '18 at 17:37

You must log in to answer this question.

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