13

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)++ ?

CS Pei
  • 10,869
  • 1
  • 27
  • 46
Adrian Suciu
  • 157
  • 1
  • 11

3 Answers3

12

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.

Brian McFarland
  • 9,052
  • 6
  • 38
  • 56
  • The original code is definitely a 'work-around' of the rules, not an 'improvement' because of them. The for loop is the right pattern. – AShelly May 14 '15 at 15:39
  • Yeah, come to think of it, I guess `memset` itself is not even MISRA compliant. So even in cases where it will work, it's not the right way per MISRA. – Brian McFarland May 14 '15 at 15:44
10

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.

embedded.kyle
  • 10,976
  • 5
  • 37
  • 56
  • 1
    Correction: MISRA-C:2012 allows ++ but not +. It is different and more relaxed than the cited rule in MISRA-C:2004. – Lundin May 18 '15 at 09:09
5

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.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • I'd judged the intention of the rule as requiring that all pointer objects identify the start of an allocation, which is a semantic limitation in other languages (such as standard Pascal). A dialect of C with such a semantic limitation would be not able to do everything that can be done in C, but an implementation of such a dialect could support array-bounds checking in ways that aren't possible in "full-powered" C. Having compiler-supported array bounds checking could be helpful in some kinds of systems that need to be fail-safe. – supercat Dec 15 '17 at 18:13