CODEWITHSUNDEEP
cmisra
Armia Wagdy
Armia Wagdy

Reputation: 599

Wraparound in unsigned arithmetic operation

The following MISRA violations appeared while analyzing the following code:

#define z  (USHORT)14745

#define Convert(x) ((((USHORT)x*(unsigned short)1000) + ((z) / (USHORT)2)) /(z))

static const USHORT array [2] = 

{
   Convert(176), -> Line "1"
   Convert(206)  -> Line "2"
};

The following MISRA violations are detected on both lines "1", "2":

My question is: Why there will be a wraparound in this operation ?!

Note: When I am checking the values of array with debugger:

array [2] = 
{
  12,
  14
}

which are the correct values.

Upvotes: 0

Views: 2366

Answers (2)

Armia Wagdy
Armia Wagdy

Reputation: 599

It is related to Integral Promotion:

Integer types smaller than int are promoted when an operation is performed on them. If all values of the original type can be represented as an int, the value of the smaller type is converted to an int; otherwise, it is converted to an unsigned int.

So that, the macro Convert(206) [((((unsigned short)206*(unsigned short)1000) + ((z) / (USHORT)2)) /(z))] will be executed as following:

  1. "206" will be promoted to signed int.
  2. "1000" will be promoted to signed int.
  3. The operation "206 * 1000" will be performed and the result will be of type signed int also -> "206000".
  4. z, 2 will be promoted to signed int.
  5. The operation z / 2 will be performed and the result will be of type signed int -> "14745 / 2" = "7372" 6."206000" + "7372" will be performed and the result will be of type signed int -> 213372
  6. "213372" will be divided by "14745" resulting "14"

Upvotes: 0

Lundin
Lundin

Reputation: 213832

First of all, 176 * 1000 will not fit inside a 16 bit unsigned short. So by using MISRA, you prevented severe bugs in your code, because the algorithm was calculated on signed int type and the result of it implicitly showed back into an unsigned short. If you get the expected results, it is entirely by coincidence/luck.

Note that there are two other advisory MISRA violations that weren't reported:

  • function-like macros are not allowed (rule 19.7)
  • you should use a pre-defined set of integer typedefs like stint.h (rule 6.3)

Both of these are very good rules and not something you should ignore. (Also it should have warned about you using literals without 'u' suffix.)

The fix is to replace the messy macro with a type safe function, that contains no implicit promotions (given 32 bit int):

uint16_t convert (uint16_t x)
{
  const uint16_t z = 14745u;
  uint32_t result32;

  result32 = (uint32_t)x * 1000ul + (uint32_t)z / 2ul / (uint32_t)z
  return (uint16_t)result32;
}

Upvotes: 1

Related Questions