PIC18F2550 Loop Counter Overwritten by sprintf

Question

I have the following code compiled with Microchip's MPLab and XC8 compiler and running on a PIC18F2550 which is doing something rather odd:

char output[20];
int i = 0;

char currentStatus = readShiftReg();
for (i = 0; i < 8; i++) {
    if ((currentStatus & (1 << i)) == (1 << i))
        sprintf(output, "Sensor %d is currently on \r\n", i + 1);
    else
        sprintf(output, "Sensor %d is currently off \r\n", i + 1);
    putsUSART(output);
}    

The for loop only ever iterates once and during that one iteration it correctly runs the second sprintf, but after it runs it, the value of i is something random (26154, 8294, ...). I've tried swapping i out for another variable j assigning the value of i to j at the beginning of the loop, but the same thing still happens to i.

It seems like it's something with sprintf because when I use the debugger, the value of i isn't changed until after sprint runs. One thing to note is that the value in output is correct (i.e. "Sensor 0 is currently off \r\n") which makes this even more perplexing.

This should be a very simple piece of code, but it's not working and I'm sure there's a simple explanation. Where should I be looking?

Answer 1

Your problem is this line:

char output[20];

You are allocating 20 characters for the buffer to hold your string, then calling sprintf() to fill it.

However your format string is too long, even before the decimal value is included. Remembering that the last array element will always need to be \x00 (a NUL character) you have 19 characters to use for your message.

Count the number of characters in this string:

Sensor %d is currently off \r\n

There are 29 characters (assuming a two digit number). This results in a buffer overflow, which will start to clobber other variables and may cause your program to operate in an unexpected manner. It is one common type of security issue in C.

Increase the size of your buffer so it is large enough to contain:

• all of the fixed letters from your message.
• the largest number the variable i will ever hold.
• the trailing \r\n.
• the terminating NUL byte.

Answer 2

Your char output declaration is too small for the string you want to write in.

Try it again with char output[50]

Your count variable i is saved directly behind the last variable of the char-array.

Something like this:

Adress       Value
0x0F00      char[0]
0x0F01      char[1]
  .           .
  .           .
  .           .
0x0F19      char[19]   <-- last variable of char
0x0F20      i          <-- counter variable

Now sprintf doesn't check the length of the array it writes to. Check the length of the array before writing it to code!