C: A cure for the warning: integer overflow in expression?

Question

I am trying to organise my UART library and prettify it a little bit by adding some #define s so I can customize it later without having to dig deeply into the code, but I can't seem to get the following bit of code working:

#define FOSC        8000000
#define BAUDRATE    9600
#define BRGVAL      (FOSC/2)/(16*BAUDRATE)-1

void uart_init(){
   U1BRG = BRGVAL;
}

After the calculation BRGVAL becomes 25.0416667, and because it is not an integer I get the following warning for it when I assign that into U1BRG:

UART.c: In function 'uart_init':

UART.c:24: warning: integer overflow in expression

...and the code simply does not work on target hardware. (If I manually put in U1BRG = 25 it works like a charm though)

Is there any way to typecast that constant into an integer to make the compiler happy?

Many Thanks, Hamza.

Answer 1

I would probably use this:

#define BRGVAL      ((int)(FOSC/2)/(16*BAUDRATE)-1)

Answer 2

What is sizeof(int) on this platform?

Answer 3

You have failed to point this out, What's the data type for U1BRG? If it's an int, cast it like as shown

#define FOSC        8000000
#define BAUDRATE    9600
#define BRGVAL      ((long)(FOSC/2)/(16*BAUDRATE)-1)

void uart_init(){
   U1BRG = BRGVAL;
}

Edit: Amended this to take into consideration of Adam Liss's comment that an unsigned int is too small to hold the result of the macro, I have changed it to make it a long...Thanks Adam for the headsup...

Hope this helps, Best regards, Tom.

Answer 4

It's not clear from your example whether U1BRG is a global variable or a #define'ed constant. In any case, simply casting to an integer should work:

 U1BRG = (int)BRGVAL;

Answer 5

Integer overflow means that you have exceeded the upper limit of an int value, which is likely to be 32767 if you are getting this error. It has nothing to do with floating point; the operations you have specified are in fact integer math operations, so the fractional part of the division is discarded anyway.

Try something like this:

#define FOSC        8000000L
#define BAUDRATE    9600L
#define BRGVAL      ((unsigned int)((FOSC/2)/(16*BAUDRATE)-1))

void uart_init(){
   U1BRG = BRGVAL;
}

The L suffix turns these constants into long type instead of int type. The (unsigned int) cast converts to U1BRG's type, and lets the compiler know that you understand that the long value will fit into an unsigned int and thus hide any warnings it may throw at you.

Normally, it's bad practice to silence compiler warnings, but in this case, it's clear that although you need long to store intermediate values in the calculation, the final result will fit into an unsigned int.

Answer 6

I like Philip's answer, but I think a better solution is to reduce the formula and change your macro to:

#define BRGVAL (FOSC/32/BAUDRATE-1)

In doing so, you eliminate the cast so the compiler can continue to warn you if you choose a low baud rate that would result in a divider value too large for a 16-bit int.