1

Why is it that if I reduce the size of the array in the code below from filename[4] to filename[3], the filename string is always 001?

char filename[4]; // if i change 4 to 3, i get filename as 001 always

while (fread(buffer, 512, 1, card) != 0)
{
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
    {
        printf("Found!!!\n");
        jpg_number++;
        sprintf(filename, "%03d", jpg_number);
    }
}
  • if you run it in gdb: is it entering the if statement correctly? what's the value of jpg_number right before the sprintf() line? (i.e.: is it actually incrementing it before writing a new filename?) – abelinux Jan 5 '15 at 20:41
  • if any post actually answered your question, please mark it "accepted" and upvote it, so future students can benefit from an already answered question. – abelinux Jan 8 '15 at 21:59
1

Your filename should be large enough to hold 3 numbers plus .jpg plus a nul char to signify the end of the string.

When you set filename[3] that is only big enough to hold 2 digits plus the nul char, so when you sprintf to it, it's not fitting so the result can be garbage.

Make sure your filename can hold the entire string (and don't forget the .jpg extension that is required!).

| improve this answer | |
1

As @curiouskiwi stated, the "right" answer would be: you're padding your string to contain 3 chars, sprintf() adds a \0 char, so total is 4, but your buffer is only 3 bytes long. Undefined behaviour is to be expected. You should always declare your buffers to contain enough chars for your strings.

Now, just for the sake of discussion, try following code:

/*
 * Testing 'sprintf()'
 */

#include <stdio.h>


int main(void)
{
    // char filename[4];  // -> works OK
    char filename[2];  // "shouldn't" work. Nonetheless...
    int i = 0;

    while (i < 100)
    {
        i++;
        sprintf(filename, "%05d", i);
        printf("filename = %s\n", filename);
    }

    // Check addresses:
    printf("&filename[0] = %p\n", filename + 0);
    printf("&filename[1] = %p\n", filename + 1);
    printf("&filename[2] = %p\n", filename + 2);
    printf("&i = %p\n", &i);

    printf("DIFF &i - &filename[2] = %ld bytes\n", (char*)&i - (filename + 2));

    return 0;
}

you'll (probably) get an output like:

filename = 00001
filename = 00002
[...]
filename = 00098
filename = 00099
filename = 00100
&filename[0] = 0x7fff008bb500
&filename[1] = 0x7fff008bb501
&filename[2] = 0x7fff008bb502
&i = 0x7fff008bb50c
DIFF &i - &filename[2] = 10 bytes

As you can see, you actually can (sometimes) step out of your buffer bounds and, if by little (and if a little lucky), you may not get any errors. This chunk of code just works!.

Of course, this doesn't mean this is correct: it just means it's a more subtle bug, which will be way more difficult to find, because your compiler won't yell back.

On the other hand, if you happened to pad your output with more zeros in a way you actually overwrite i (remember we just saw, in the output, that i is 10 bytes away from filename[2]):

sprintf(filename, "%012d", i);

Your code would compile, but when you actually run it, you'd get yourself into an infinite loop.

Can see why? Every time through the loop, i gets incremented, but then sprintf overwrites it with zeros. Nonetheless i is always pointing at the same mem location, so when you repeat the loop and try to increment it, the current value of i is always 0, therefore the incremented value is always 1.

Of course, the infinite loop doesn't happen in your code because your while condition is independant of jpg_number. But I'm pretty confident you'll find in a situation like this one if you run your code through gdb (that's why my previous comment to your question).

| improve this answer | |

You must log in to answer this question.

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