Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Rule 10.5
#1
First of all, in the explaining text to Chapter 6.10 you can read the following below \"Balancing conversions\":

\"Notice that the operands of the bitwise shift operators (>) are not balanced. The type of the result is the promoted type of the first operand.\"

The balancing refers to the \"usual arithmetic conversions\" and I believe that the text is correct.

Then in the example for rule 10.5 the following code is posted:

Code:
uint8_t port = 0x5aU;
uint8_t result_8;
uint16_t result_16;

result_8 = (~port) >> 4;  /* not compliant */
...
result_8 = ((uint8_t)(~port)) >> 4; /* compliant */
result_16 = ((uint16_t)(~(uint16_t)port)) >> 4; /* compliant */

The last line with result_16 is incorrect and not compliant to rule 10.5 nor rule 10.3. The result of that line will be 0x0FFA and not 0x000A.

The rule 10.5 states that the result of an expression shall be immediately cast to the underlying type. Underlying type is defined as the type an expression would have if not for the integer promotions. If not for the integer promotions, the type of the expression (~port) would be uint8_t and the type of the expression (~port) >> 4 would be also be uint8_t, since the usual arithmetic conversions aren't used, see the text on top of this post.

First error in that line:

~(uint16_t)port

The operand is casted instead of the result.

Second error on that line:
The result is not casted to the underlying type which is uint8_t.

Third error on that line:
result_16 = (uint16_t) ...

The result is not casted to the underlying type of the expression
\"uint8_t result\" >> 4
which is uint8, since the usual arithmetic conversions aren't used on shift operators.

So not only does the line break rule 10.5, it also breaks rule 10.3 in MISRA C:2004 TC1: \"The value of a complex expression of integer type shall only be cast to a type of the same signedness that is no wider than the underlying type of the expression\".

I believe that the correct line will look like this:

Code:
result_16 = (uint8_t) ( ((uint8_t)~port) >> 4U ); /* compliant */
However, that line looks quite horrible. I suggest that the following code is used instead:

Code:
result_8  = (uint8_t) ~port;
result_16 = (uint8_t) (result_8 >> 4U);
#2
Doesn't this whole issue show up the weakness of the MISRA-2004 (and earlier) approach?

MISRA argues that many problems arise because of \"misunderstanding and difficulty in the C language\"/\"Misconceptions among programmers\". Instead of attacking the root cause, it creates an unwieldy set of rules designed to \"neutralize [the effect of integral promotion] by taking no advantage of the widening that occurs with small integer operands\". The number of casts required obfuscate the code, and weaken the compiler's static type checking: the compiler cannot detect an improperly placed cast.

One idea to eliminate \"misconceptions among programmers\" is a training & certification program. The issues are limited -- they represent only a handful of pages in the standard -- so such a program could be largely automated, e.g., via the Web.

Regards,

Konrad Schwarz
<t>Konrad Schwarz<br/>
Siemens AG<br/>
CT SE 2 (Corporate Technology, Software & Engineering - Embedded Systems)</t>
#3
1) The context of the example code may be a little confusing but the code IS compliant. Yes - the value assigned to result_16 result will be 0x0FFA. It is not intended to have the same value as result_8.

2) The operand of the ~ operator is "(uint16_t)port" and the underlying type of this operand is uint16_t because the underlying type of the result of a cast is the same as the type of the cast.

3) The result of the ~ operation is subsequently cast to the underlying type as in ... "(uint16_t)(~(uint16_t)port)"
Posted by and on behalf of the MISRA C Working Group
#4
1) In the text above that particular example we can read: "In either case the value of result is 0xfa, but 0x0a may have been expected. This danger is avoided by inclusion of the cast as shown below:"

As I can see it, that text really leaves little room for other results than 0x0a, I would have expected the example not to give the result 0xfa nor 0x0ffa, which is the danger that the text warns us about.


2 + 3) If the example indeed intends to give the very confusing result 0x0FFA, then I assume that the cast is there as a futile attempt to ensure that the ~ operation is made on uint16_t rather than uint8_t. That is, I assume that the typecast is not added as an attempt to conform with rule 10.5, but is rather part of the original code written by someone who is not aware of the integer promotions. The variable port will first be explicitly typecasted to uint16_t, and then before the ~operator is executed, "port" will either remain uint16_t (on a 16-bit machine where int is 16 bit) or it will be implicitly typecasted to the type "int" by the integer promotions. In either case, the explicit typecast to uint16_t is completely pointless. The signedness of int is not important here, since a final typecast of the result is made later.

If the explicit cast is there as a failed attempt to make "port" 16 bit, and is not something that was added as an attempt to conform with rule 10.5, yes in that case the code would indeed be compliant, if not very intuitive. The cast is very confusing for the reader, since the chapter is there to enlighten people of the dangers with implicit typecasts.

Also, if the average programmer is writing code that appends 0xFF in front of a value, they do not usually write something that relies on the size of int to implicitly add a arbitrary number of bytes with the value 0xFF to the variable through the ~ operator. They write something readable:

Code:
uint16_t port16;

  port16 = (uint8_t) ~port;
  port16 |= 0xFF00U;
  port16 >>= 4U;
<t></t>
#5
It is acknowledged that the code examples are somewhat contrived and potentially confusing, but they are correct. Clarifications will be made in a future revision of the MISRA-C document."
[ID:0000007]
Posted by and on behalf of the MISRA C Working Group


Forum Jump:


Users browsing this thread: 3 Guest(s)