CODEWITHSUNDEEP
cpointersembeddedmisra
Adrian Suciu
Adrian Suciu

Reputation: 157

MISRA incrementation in C

While debugging some embedded code, I came across something like this:

buffPtr = &a[5];
buffEndPtr = &a[10];

while (buffPtr != buffEndPtr) 
{ 
    *buffPtr = 0xFF; 
    buffPtr  = &buffPtr[1];         /*  MISRA improvement for: buffPtr++ */ 
}

Why would this construct be an improvement over (*buffPtr)++ ?

Upvotes: 13

Views: 10471

Answers (3)

Lundin
Lundin

Reputation: 214780

In MISRA-C:2004 rule 17.4 there was an advisory rule banning all forms of pointer arithmetic. The intent was good, the purpose of the rule was an attempt to ban potentially dangerous code such as: 

stuff* p; 
p = p + 5; // 5 stuff, not 5 bytes, bug or intentional?

and hard-to-read code such as

*(p + 5) = something;  // harder to read but equivalent to p[5]

Generally, the intention is to recommend using an integer iterator instead of pointer arithmetic, when looping through pointed-to data.

However, the rule also banned various fundamental pointer operations that are likely not dangerous, for example ptr++. Generally, the rule was far too strict.

In MISRA-C:2012 this rule (18.4) was relaxed to only ban the + - += -= operators.

In your case, the buffPtr = &buffPtr[1]; was a misguided attempt to dodge rule 17.4, because the rule didn't make much sense. Instead the programmer decided to obfuscate their program, making it less readable and therefore less safe.

The correct way to deal with this would be to use the ++ operator and ignore rule 17.4. It is an advisory rule, so no deviation needs to be done (unless the local MISRA-C implementation for some reason says otherwise). If you do need to deviate, you could simply say that the rule doesn't make any sense for the ++ operator and then refer to MISRA-C:2012 18.4.

(Of course, rewriting the whole loop to a for loop as shown in another answer is the best solution)

Programming without using common sense is always very dangerous, as is blindly following MISRA without understanding the sound rationale behind the rules, or in this case the lack of such.

Upvotes: 5

Brian McFarland
Brian McFarland

Reputation: 9432

There is a MISRA rule that states the only pointer math allowed is the indexing operation.

The pattern you have shown is a poorly executed work-around. It is ugly/weird/uncommon and probably based on a misunderstanding of the purpose of that rule. It may also violate another rule.

A better way to write this code would be:

for(i=5; i < 10; i++)
{
    a[i] = 0xff;
}

Update 2015-05-20 - Since this was the accepted answer here's the actual rule violated, courtesy of embedded.kyle:

MISRA-C:2004, Rule 17.4 (Required) or MISRA-C:2012, Rule 18.4 (Required) Array indexing shall be the only allowed form of pointer arithmetic.

Upvotes: 12

embedded.kyle
embedded.kyle

Reputation: 11466

The rule that (*buffPtr)++ is violating is:

MISRA-C:2004, Rule 17.4 (Required) or MISRA-C:2012, Rule 18.4 (Required)

Array indexing shall be the only allowed form of pointer arithmetic.

Their reasoning behind this rule:

Array indexing using the array subscript syntax, ptr[expr], is the preferred form of pointer arithmetic because it is often clearer and hence less error prone than pointer manipulation. Any explicitly calculated pointer value has the potential to access unintended or invalid memory addresses. Such behavior is also possible with array indexing, but the subscript syntax may ease the task of manual review.

Pointer arithmetic in C can be confusing to the novice The expression ptr+1 may be mistakenly interpreted as the addition of 1 to the address held in ptr. In fact the new memory address depends on the size in bytes of the pointer's target. This misunderstanding can lead to unexpected behaviour if sizeof is applied incorrectly.

Many of MISRA's rules have similar rationales. Basically their thought process is that if you write as simplistically and explicitly as possible, the code will be more readable and maintainable, which would therefore lead to inherently safer code. Safer code is the purpose behind the MISRA standard.

As Brian pointed out, there are ways to write code that are MISRA compliant but still violate the intention behind the rule. Brian's for loop example would be the most common and easily understandable construct in my opinion.

Upvotes: 10

Related Questions