Search code examples
cmisra

MISRA-C 2012 Rule 10.8 Query

I am getting MISRA-C 2012 Rule 10.5 voilation, below is the sample code :

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

typedef long long       sint64; 
typedef unsigned long long  uint64;
typedef unsigned long   uint32;

#define ntohll(x) ( ( (uint64)(ntohl( ((x << 32) >> 32) )) << 32) | ntohl( ((uint32)(x >> 32)) ) )

void main()
{
 sint64 pul_total;
 sint64 a;
 pul_total = ntohll(a); /* Rule 10.8 Violation*/    
}

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ to resolve the issue I tried below :

#define ntohll(x) ( ( (uint64)(ntohl( ((x << 32) >>(uint32)32) )) << (uint32)32) | ntohl( ((uint32)(x >>(uint32) 32)) ) )

but still its a voilation

However if I make it like below violation is removed :

  #define ntohll(x) ( ( (uint64)(ntohl( ((x << 32) >> 32) )) << 32) | ntohl( ((uint32)((uint32)x >> 32)) ) )

but as per my understanding casting signed variable to unsigned might not be a good idea in case of shift operation.

Need some help for the same...

Solution

  • This whole code is definitely not MISRA-C compliant.

    • First of all there's some less important nit-picks. The Directive 4.9 is saying that function-like macros should be avoided entirely. And rule 7.2 is saying that you must use u suffix on all integer constants.

    • What's most serious here is the violation of 10.1 which says "Shift and bitwise operations should only be performed on operands of essentially unsigned type".

      You left shift a signed operand - if that operand is negative your code invokes undefined behavior and you have a severe bug. You then also right shift a signed operand, which invokes implementation-defined behavior if the operand is negative. These are not just some false positives but actual bugs you must fix. The easiest fix is to cast x to uint64_t before any shifting.

    • I see no violation of 10.5, which would be casting to an inappropriate type. It is fine to cast from signed to unsigned.

    • There is however a violation of 10.8 as your comment indicates - the rule doesn't allow the result of a "composite expression" to be cast to a different type category, in your case from sint64_t to uint64_t or uint32_t. This too could be solved by casting to uint64_t before doing anything else.

      The (rather weird) rationale for 10.8 is that some beginners supposedly think that a cast such as (uint32_t)(u16a + u16b); means that the + operation gets carried out on uint32_t, which is incorrect.

    Now the real question is what all this shifting is actually trying to achieve to begin with; it isn't clear to me. The macro is quite messy. If the intention was to clear out some bits of a variable, that should be done with bit masking (bitwise &). And if reasons unknown signed variables must be used and sign must be preserved, the bit mask can simply skip the sign bit.

    The best way to fix this code is to rewrite that macro entirely. As it stands, it will never pass MISRA-C, which is a good thing.