Unexpected result when calculating a percentage - even when factoring in integer division rules

Question

I am trying to express a battery voltage as a percentage. My battery level is a (global) uint16 in mV. I have a 16-bit CPU. Here is my code:

static uint8 convertBattery(void){
    uint16 const fullBattery = 3000; /* 3V = 3000mV */
    uint8 charge;
    charge = ((battery*100)/fullBattery);
    return charge;
}

As you can see, I have factored in the integer division rounding down by multiplying the numerator by 100 first.

With a battery = 2756 my charge values calculates as 04 which is unexpected. I've spent ages on this rather trivial task and haven't made any progress. Could anyone suggest where the problem is?

Thanks.

Answer 1

Diagnosis

The value you expect is, presumably, 91.

The problem appears to be that your compiler is using 16-bit int values.

^{You should identify the platform on which you're working and include information about unusual situations such as 16-bit int types. It is reasonable for us to assume 32-bit or 64-bit systems by default — they are by far the most common environments for questions on SO.}

You have battery = 2756. If that is multiplied by 100, it gives you 275600 in 32-bit precision, but in 16-bit precision, it gives 13546, which, when divided by 3000, yields 4, the observed answer.

Here's code compiled with a 64-bit compiler that simulates the 16-bit calculation:

#include <stdio.h>

int main(void)
{
    short battery = 2756;
    short fullbattery = 3000;
    short r1 = battery * 100;
    printf("r1 = battery * 100 = %d\n", r1);
    short r2 = r1 / fullbattery;
    printf("r2 = (battery * 100) / fullbattery = %d\n", r2);
    int r3 = (battery * 100) / fullbattery;
    printf("r3 = (battery * 100) / fullbattery = %d\n", r3);
    return 0;
}

The output from it is:

r1 = battery * 100 = 13456
r2 = (battery * 100) / fullbattery = 4
r3 = (battery * 100) / fullbattery = 91

To get a 16-bit calculation, I had to force the intermediate results into 16-bit variables. Yes, there are other more elaborate ways of writing the code, using uint16_t etc. And yes, the overflow in the assignment to r1 is strictly undefined behaviour, but my compiler (GCC 5.1.0 on Mac OS X 10.10.3) appears to be giving a sane interpretation to the undefined behaviour. This serves to demonstrate the point.

Plausible fix

You should be able to work around it on your machine by using long:

static uint8 convertBattery(void){
    uint16 const fullBattery = 3000; /* 3V = 3000mV */
    uint8 charge = (battery * 100L) / fullBattery;
    return charge;
}

The L in 100L makes it a long value, which must be at least 32-bits in a conforming C compiler, so the multiplication must give a long value, and the divisor will be converted to long before the division, and the result will be a long (and the value will be 91), and you can assign that to charge safely.

Alternatively, you could use one or more explicit (long) casts in the calculation if you think this is too subtle.

Answer 2

The intermediate result of battery*100 is too large for an uint16, so it overflows.