My today's C gotcha

Go To Last Post
13 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have a menu - several strings, an array of pointers for that strings, and an index to that array. I also have a function returning the movement of an encoder since last call of the function (a signed number). Something along these lines:

char a1[] = "Menu1";
char a2[] = "Menu2";
char a3[] = "Menu3";
char a4[] = "Menu4";
char a5[] = "Menu5";
char a6[] = "Menu6";
char a7[] = "Menu7";
char a8[] = "Menu8";
char a9[] = "Menu9";

char * menu[] = {a1, a2, a3, a4, a5, a6, a7, a8, a9};

volatile uint8_t menuIndex;

#define SIZE_A (sizeof(menu) / sizeof(char *))

int16_t EncoderMovement(void);  // defined elsewhere

void ProcessMenu(void) {
  int16_t tmp;

  tmp = EncoderMovement();
  tmp = menuIndex + tmp;
  tmp = tmp % SIZE_A;
  if (tmp < 0) tmp = tmp + SIZE_A;
  menuIndex = tmp;
  DisplayMenu(tmp);
}

When Menu1 is displayed and I move the encoder one tick backwards, it jumps onto Menu6; moving it further backwards it goes nicely to Menu5-Menu4...Menu1 when it jumps again to Menu6; moving it forward goes as supposed to Menu7 (then Menu8-Menu9-Menu1-Menu2...

Hands up those who know right off where's the gotcha.

I supposedly should be ashamed for this kind of error:

Derek Jones wrote:
Developers forgetting about the [root cause of my problem] is not something that can be addressed by a guideline recommendation.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Hands up those who know right off where's the gotcha
I did not see it right off. But I have an excuse - lately I almost never use signed types. (Well, except in Java, of course.)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As you wrote it you assume that

sizeof (int16_t) == 0 mod SIZE_A

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

SprinterSB wrote:
As you wrote it you assume that
sizeof (int16_t) == 0 mod SIZE_A

I don't understand. Please, explain.

Jan

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry for the confusion, I meant

2^sizeof (int16_t) == 0 mod SIZE_A

of course, where ^ stands for POW.

Thus if you have negative number and use unsigned modulo, you have wrap around which adds 2^16 mod SIZE_A to your result.

@Admins: Is there way to show formulae that are more complex than "1+1", for example something like math-Tags to render TeX?

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:

char a1[] = "Menu1";
char a2[] = "Menu2";
char a3[] = "Menu3";
char a4[] = "Menu4";
char a5[] = "Menu5";
char a6[] = "Menu6";
char a7[] = "Menu7";
char a8[] = "Menu8";
char a9[] = "Menu9";

char * menu[] = {a1, a2, a3, a4, a5, a6, a7, a8, a9};

volatile uint8_t menuIndex;

#define SIZE_A (sizeof(menu) / sizeof(char *))

int16_t EncoderMovement(void);  // defined elsewhere

void ProcessMenu(void) {
  int16_t tmp;

  tmp = EncoderMovement();
  tmp = menuIndex + tmp;
  tmp = tmp % SIZE_A;
  if (tmp < 0) tmp = tmp + SIZE_A;
  menuIndex = tmp;
  DisplayMenu(tmp);
}

When Menu1 is displayed and I move the encoder one tick backwards, it jumps onto Menu6; moving it further backwards it goes nicely to Menu5-Menu4...Menu1 when it jumps again to Menu6; moving it forward goes as supposed to Menu7 (then Menu8-Menu9-Menu1-Menu2...

signed % unsigned causes the signed to be "promoted" to unsigned.
That has the effect of adding 2**16 to the signed.
Quote:
I supposedly should be ashamed for this kind of error:
I don't know why.
To me, that would seem a frequent use of % and deserving of a warning.

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah, you mean that I assumed that 65536 (let's talk specifically avr-gcc) is divisible by SIZE_A without a remainder?

This can be a certain view on the problem but IMHO the root cause is different. It is the "usual arithmetic conversions" rules which are IMHO quite conter-intuitive in many cases, especially if I "forget" that result of sizeof is of unsigned type (as Stefan mentioned in the other thread).

Moreover, how do you define "unsigned modulo"? In C, (a/b)*b + a% b = a, thus -1/9 = 0 and -1% 9 = -1 ... I'd call that "signed modulo", just being performed on operands which were converted to unsigned due to the rules above.

JW

PS. Apparently there are no admins who would care about the technicalities of this site, see the % problem. But I am glad it is here as it is.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
This can be a certain view on the problem but IMHO the root cause is different.
So if you insist on unsigned mod, you will need to treat thw two cases anyway. I don't think it's better than signed modulo. And sined modulo is better the grasp than doing distinction of cases. Just write
tmp = tmp % (int16_t) SIZE_A;

Notice that if you have wrap-around in other places, e.g. addition, you will likewise see a 2^16 mod SIZE_A out of the blue.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
Ah, you mean that I assumed that 65536 (let's talk specifically avr-gcc) is divisible by SIZE_A without a remainder?
No.
You seem to have expected that the arithmetic would be done with the given numbers.
Because of standard "promotions" that didn't happen this time.
Quote:
This can be a certain view on the problem but IMHO the root cause is different.How do you define "unsigned modulo"? In C, (a/b)*b + a% b = a, thus -1/9 = 0 and -1% 9 = -1.
In C, -1 % 9 != -1 % 9U, even if -1 % 9 is positive.

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:
wek wrote:
Ah, you mean that I assumed that 65536 (let's talk specifically avr-gcc) is divisible by SIZE_A without a remainder?
No.
You seem to have expected that the arithmetic would be done with the given numbers.
Because of standard "promotions" that didn't happen this time.
Yes - and that's the gotcha.

Michael wrote:
Quote:
This can be a certain view on the problem but IMHO the root cause is different.How do you define "unsigned modulo"? In C, (a/b)*b + a% b = a, thus -1/9 = 0 and -1% 9 = -1.
In C, -1 % 9 != -1 % 9U, even if -1 % 9 is positive.
In C, -1 % 9 is not positive, and that's why I'd not call the modulo operation "unsigned". However, I see why Johann calls it usigned, from his point of view as the language developer he needs to perform different operations when the operands are signed versus unsigned.

More rants.

The gotcha conversion happens only under certain circumstances, see "usual arithmetic conversions" in C99. Just for some extra fun, I tried the following:

#include 
#include 
int main(void) {
  printf("%d %d %d %d  \n %d %d %d %d  \n %d %d %d %d  \n %d %d %d %d", 
         (int8_t)-1 % (uint8_t)9, 
         (int8_t)-1 % (uint16_t)9, 
         (int8_t)-1 % (uint32_t)9,
         (int8_t)-1 % (uint64_t)9,

         (int16_t)-1 % (uint8_t)9, 
         (int16_t)-1 % (uint16_t)9, 
         (int16_t)-1 % (uint32_t)9,
         (int16_t)-1 % (uint64_t)9,

         (int32_t)-1 % (uint8_t)9, 
         (int32_t)-1 % (uint16_t)9, 
         (int32_t)-1 % (uint32_t)9,
         (int32_t)-1 % (uint64_t)9,

         (int64_t)-1 % (uint8_t)9, 
         (int64_t)-1 % (uint16_t)9, 
         (int64_t)-1 % (uint32_t)9,
         (int64_t)-1 % (uint64_t)9
);
  exit(0);
}

Try (without cheating) to predict the results for a 32-bit compiler and for a 16-bit compiler. Very enlightening.

At this point, I see little reason to stop hating C.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
exit(0);
Should not this be "exit(EXIT_SUCCESS);"? Oh wait, the standard allows a 0. Never mind. I just don't like seeing a 0 there. Oh, well. Sorry for hijacking your thread.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
Michael wrote:
Quote:
This can be a certain view on the problem but IMHO the root cause is different.How do you define "unsigned modulo"? In C, (a/b)*b + a% b = a, thus -1/9 = 0 and -1% 9 = -1.
In C, -1 % 9 != -1 % 9U, even if -1 % 9 is positive.
In C, -1 % 9 is not positive,
In C89, the sign of -1 % 9 is implementation defined.
Quote:
Just for some extra fun, I tried the following:

// comments added by skeeve
#include 
#include 
int main(void) {
  printf("%d %d %d %d  \n %d %d %d %d  \n %d %d %d %d  \n %d %d %d %d",
         (int8_t)-1 % (uint8_t)9, // -1, -1
         (int8_t)-1 % (uint16_t)9, // -1, bad
         (int8_t)-1 % (uint32_t)9, // bad, bad
         (int8_t)-1 % (uint64_t)9, // bad, bad

         (int16_t)-1 % (uint8_t)9, // -1, -1
         (int16_t)-1 % (uint16_t)9, // -1, bad
         (int16_t)-1 % (uint32_t)9, // bad, bad
         (int16_t)-1 % (uint64_t)9, // bad, bad

         (int32_t)-1 % (uint8_t)9, // -1, -1
         (int32_t)-1 % (uint16_t)9, // -1, -1
         (int32_t)-1 % (uint32_t)9, // bad, bad
         (int32_t)-1 % (uint64_t)9, // bad, bad

         (int64_t)-1 % (uint8_t)9, // -1, -1
         (int64_t)-1 % (uint16_t)9, // -1, -1
         (int64_t)-1 % (uint32_t)9, // -1, -1
         (int64_t)-1 % (uint64_t)9  // bad, bad
);
  exit(0);
}

Try (without cheating) to predict the results for a 32-bit compiler and for a 16-bit compiler. Very enlightening.

Comments in code added by skeeve.
Note that d is the wrong format for most of the expressions,
so "-1, -1" followed by garbage is a possibility.

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:
Comments in code added by skeeve.
Note that d is the wrong format for most of the expressions,
so "-1, -1" followed by garbage is a possibility.
Correction: Note that d is the wrong format for about half of the expressions,
so "-1, -1" followed by garbage is a possibility.

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods