5
votes

What can I do to avoid MISRA giving this error for the code below? I tried casting with (unit16_t). But then it didn't allow an explicit conversion.

Illegal implicit conversion from underlying MISRA type "unsigned char" to "unsigned int" in complex expression (MISRA C 2004 rule 10.1)

 uint8_t rate = 3U; 
 uint8_t percentage = 130U;      
 uint16_t basic_units = rate * percentage;
4

4 Answers

5
votes

The problem is that both rate and percentage are silently promoted by the integer promotions to type "int". The multiplication is therefore performed on a signed type.

MISRA compatible code is to either rewrite the code as

 uint16_t basic_units = (uint16_t)rate * (uint16_t)percentage;

or do as MISRA suggests, immediately typecast the result of an expression to its "underlying type":

 uint16_t basic_units = (uint8_t)(rate * percentage);

EDIT: Clarification follows.

ISO 9899:1999 6.3.1.1 2

If an int can represent all values of the original type, the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions.

Informative text from MISRA-C:

MISRA-C:2004 6.10.3 Dangerous type conversions:

/--/

- Change of signedness in arithmetic operations: Integral promotion will often result in two unsigned operands yielding a result of type (signed) int. For example, the addition of two 16-bit unsigned operands will yield a signed 32-bit result if int is 32 bits but an unsigned 16-bit result if int is 16 bits.

I'm actually not sure whether the 2nd line of mine above would satisfy MISRA or not, at second thought I may have confused MISRA 10.1 with 10.5, where the latter enforces an immediate cast to underlying type, but only in case of certain bitwise operators.

I tested both lines with LDRA static code analysis and it didn't complain (but gives some incorrect, non-related warnings), but then LDRA also performs very poorly at MISRA-C.

Anyway, the problem in the original question is that rate and percentage are both implicitly converted by the integer promotions to type int which is signed, since int can represent all values of a uint8_t. So it becomes

uint16_t basic units = (int)rate * (int)percentage.

To prevent this you have to typecast explicitly. After giving it more thought, I'd go with the 1st line of my two above.

2
votes

The implicit conversion is performed before the multiplication, for the multiplication. Maybe an explicit conversion right before the multiplication shuts up your tool

uint16_t basic_units = (unsigned)rate * (unsigned)percentage;

The resulting unsigned value, should be implicitly converted to uint16_t with no warnings. If your tool chooses to be a PITA about this too, try another explicit conversion:

uint16_t basic_units = (uint16_t)((unsigned)rate * (unsigned)percentage);
1
votes

The MISRA rule is trying to ensure that the "underlying type" used for calculation is the same as the resulting type. To achieve that, you can cast the one, or both, of the operands:

uint8_t rate = 3U; 
uint8_t percentage = 130U;      
uint16_t basic_units = (uint16_t)rate * percentage;

On a 32-bit architecture the result without the cast is OK, however, consider the following:

uint32_t rate =  ...;
uint32_t percentage = ...;
uint64_t basic_units = rate * percentage;

On a 32-bit architecture, the operation will be performed in 32 bits - even though the target type is 64 bits wide. Where rate and percentage are large enough, this could result in the operation wrapping in 32 bits and so data that would have fit in the target type will be lost.

The MISRA rule is attempting to make the code safer irrespective of the size of the types on the target platform.

0
votes

If you try casting your operands using C-style casting, you might just give your tool something else to complain about. So, instead of doing this:

uint16_t basic_units = (uint16_t)rate * (uint16_t)percentage;

You may need to do this:

uint16_t basic_units = static_cast<uint16_t>(rate) * static_cast<uint16_t>(percentage);