C unsigned modulus causes compiler warning

Question

In some embedded C code for the MC9S12C32 microcontroller, I have a circular queue (aka circular buffer) implemented with a statically sized byte array and two "pointers" for the front and rear of the queue, which are in reality just indices for the queue's array.

// call unsigned chars bytes
typedef unsigned char byte;
byte trear  = 0;   // SCI transmit display buffer IN index
byte tfront = 0;   // SCI transmit display buffer OUT index
byte tsize  = 16;  // size of transmit buffer
byte tbuf[16]= {0};// SCI transmit display buffer

Note that trear is the actual index of the rear element, but tfront is one less than actual index of the front element (subject to modulo 16 of course). So, for example, if my buffer contained "hello", it might look like this (where empty slots are garbage values):

_________________________________
| | |h|e|l|l|o| | | | | | | | | |
   ^         ^
 front      rear

When it's time to remove a byte from the queue, I do this:

// increment front index
tfront++;
// wrap front index if it exceeded bounds
tfront %= tsize;                           // (A)
// get character to transmit
byte outputChar = tbuf[tfront];

This all works fine -- at least, my program has exhibited no bugs related to this fragment. However, when I compile this program, my compiler warns me about the line marked (A) in the fragment above, complaining:

Warning : C2705: Possible loss of data

main.c line 402

Line 402 is line (A). I should note that I am not using gcc or the like; I am compiling in Freescale's CodeWarrior IDE, which has sometimes given me other somewhat mystifying warnings. In an attempt to get rid of the warning, I rewrote the fragment above as:

// increment front index mod tsize
tfront = (tfront + 1 >= tsize) ? 0 : tfront + 1;      // (B)
// get character to transmit
byte outputChar = tbuf[tfront];

However, my compiler still emits the same warning, this time about line (B). Maybe the compiler is telling me that in the statement (tfront + 1 >= tsize), tfront might be 255 before execution, and overflow. Of course, I know this won't happen, but my compiler doesn't.

If this is the case, though, why was line (A) an issue? Basically, I'd like to know what the compiler is unhappy about.

---

Since typing out my question, I've solved it by changing tsize from a variable type to a preprocessor definition (i.e., #define TSIZE 16). My question still stands, though.

---

Some related questions:
unsigned overflow with modulus operator in C
modulus operator with unsigned chars

Answer 1

The compiler warning likely comes from the fact that in tfront %= tsize;, which is equivalent to tfront = tfront % tsize;, because of promotion rules in C the expression tfront % tsize has(*) type int.

It may silence the compiler if you write tfront = (byte)(tfront % tsize); instead.

There is no particular reason to worry, and your compiler emits strange warnings indeed: although the expression tfront % tsize technically has type int, its values all fit in a byte because of the way it is computed. Even if the values did not all fit in a byte, the wrap-around behavior is guaranteed by the C standard for unsigned integer types (so that you would be justified in using this wrap-around behavior on purpose).

(*) unless on your compilation platform int cannot contain all values that an unsigned char can take, in which case it would be of type unsigned int and you probably wouldn't see the warning.

Answer 2

This particular warning is a known bug in all of the Codewarrior compilers. The possible loss of data warning is inconsistent and buggy. Sometimes it warns because there is a risk of implicit type promotions, sometimes it does not. See this discussion. I can confirm that this is also true for CW for S12C (at least as far as version 5.1, which is what I am using).

You can disable this warning, but I wouldn't recommend it, since it does find dangerous code sometimes, along with the false warnings. As in your specific case: the warning is correct.

You are doing arithmetic on small integer types, without explicit casts. Such code is dangerous and possibly contains hidden bugs, because C promotes small integers to signed ints. A coding standard like MISRA-C, which enforces explicit casts, would have helped here.

Furthermore, the ?: operator can also be dangerous unless you know how it works. It balances the 2nd and 3rd operand against each other, which one perhaps did not expect.

I would therefore suggest changing the code to:

tfront = (byte)(tfront % tsize); 
...

if( (byte)(tfront +1) >= tsize)
{
  tfront = 0;
}
else
{
  tfront++;
}