C musing of the day: signed ints

I ran across some code today that puzzled me. It was an infinite loop that used a counter to determine if things took too long. Something like this:

int main()
{
  int count;
  int status;

  count = 0;

  do
  {
    status = GetStatus();

    count++;

  } while( status == 0 );

  if (count < 0)
  {
    printf("Time out! count = %d\n", count);

    return EXIT_FAILURE;
  }

  printf("Done. count = %d\n", count);

  return EXIT_SUCCESS;
}

The code would read some hardware status (“GetStatus() in this example)  and drop out of the do/while loop once it had a value. Inside that loop, it increments a count variable. After done, it would check for “count < 0” and exit with a timeout if that were true.

Count is only incremented, so the only way count could ever be less than zero is if it incremented so high it rolled over. With a signed 8-bit value (int8_t), you count from 0 to 127, then it rolls over to -128 and counts back up to 0.

So with an “int” being a 32-bit value (on the system I am using), the rollover happens at 2147483647.

And that seems like quite a weird value.

But I suppose it it took that long, it certainly timed out.

I think if was going to do it, I would have probably used an unsigned integer, and just specified an appropriate value:

unsigned int count;

...

if (count > 10000)
{
  printf("Time out! count = %u\n", count);
  return EXIT_FAILURE;
}

What says you?

5 thoughts on “C musing of the day: signed ints

  1. James Jones

    If it can actually go 2^31 times around before status changes, why not 2^32 times, in which case the timeout check would fail? (Or would it? Signed integer overflow is officially undefined behavior, giving the compiler the right to generate code to order pizza with anchovies or launch the nuclear warheads.) Better to do something like

    count = min(MAX_COUNT, count + 1);

    and then check for count == MAX_COUNT.

    Reply
  2. Johann Klasek

    The whole structure looks weired to me. The loop for checking the status will not be left if the status remains 0. Count may wrap multiple times and finally has a meaningless value.
    As previously mentioned a minimum function should help to get rid of it. Anyway, why not to leave the status loop if the timeout is reached? Something like

    while (status == 0 && count < 10000)
    if (status == 0) /* timeout */ ….

    I wouldn't trust the hardware to ever change the status in terms of reliable software.

    Reply
    1. Allen Huffman Post author

      And that’s actually a related problem. This is based on Ethernet code and it would get stuck in that loop forever if it got PoE but no IP address. And it’s a commercial IP stack.

      Reply

Leave a Reply