-4

I am facing a MISRA C 2004 violation of rule 1.2 "likely use of null pointer. The code that I am using is as below:

tm_uint8* diagBuf = 0u;
diagBuf[0] = diagBuf[0] + 0x40u; 
diagBuf[2] = 0x01u;
diagBuf[0] = diagBuf[0] + 0x40u;
diagBuf[2] = 0x01u;

This is just a part of the code that is indicated above. some of the statements have "IF" conditions.

Can some one point out why I get the MISRA Violation.?

Ankit Shah
  • 97
  • 1
  • 11
  • Obviously, you use a null pointer. `diafBuf` points to address 0. – Andrejs Cainikovs Mar 14 '18 at 10:44
  • 1
    Best just send it back to the developer and say "MISRA C rule 1.2 violation - fix it". – Paul Ogilvie Mar 14 '18 at 10:52
  • @Pras isn't it also a MISRA rule to not use malloc? – M.M Mar 14 '18 at 10:53
  • @AndrejsCainikovs null pointers don't "point to address 0" in general – M.M Mar 14 '18 at 10:54
  • @M.M yes, you are right. So in that what should I do, The developer says it compiles without error and so now its my job to solve the MISRA violations – Ankit Shah Mar 14 '18 at 10:54
  • 1
    *"likely use of null pointer"* - That made me chuckle – StoryTeller - Unslander Monica Mar 14 '18 at 10:55
  • @AnkitShah if this is the real code then it is bugged and the developer should fix it. Your comments suggest that this isn't the real code though. You should post the real code – M.M Mar 14 '18 at 10:56
  • @M.M In general, yes. But I'm from embedded world, so there might be some rare occasions. – Andrejs Cainikovs Mar 14 '18 at 10:57
  • Doesn't MISRA prohibit assignment of (zero) integer liters to pointers in general? – user7860670 Mar 14 '18 at 11:17
  • Unfortunately MISRA doesn't know about embedded environments where you really might need to access address 0. – Gerhardh Mar 14 '18 at 11:25
  • Maybe look at line 1 of your own code for 1 second and you'll see why you get the warning... – Lundin Mar 14 '18 at 12:19
  • @Gerhardh MISRA knows about embedded environments just fine. You should study the rule of "simple assignment", 6.5.16.1, which states how assignment is done in the C language. The line `tm_uint8* diagBuf = 0u;` _only_ means that the pointer is a null pointer. It can never mean that the pointer points at address zero. This is because assignment from integer to pointer is invalid C, in every single case except when the integer is a null pointer constant. To point at address zero, you will have to write `type* ptr= (type*)0;`, which is not a null pointer constant unless type happens to be `void`. – Lundin Mar 14 '18 at 12:27
  • I posted the above as an answer, since simple assignment and null pointer constants might not be so trivial to everyone, after all. – Lundin Mar 14 '18 at 12:52

3 Answers3

1

According to the 1999 C standard, Section 6.3.2 "Pointers", para 3

An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function.

(Note I've removed cross reference at the end of the first sentence in the above to a footnote which explains that NULL is defined in <stddef.h> and other headers as a null pointer constant).

This means that

tm_uint8* diagBuf = 0u;

initialises diagBuf using a null pointer constant, since 0u is an integer constant expression with value zero. Accordingly, diagBuf is initialised as a null pointer.

Furthermore the following statements

diagBuf[0] = diagBuf[0] + 0x40u; 
diagBuf[2] = 0x01u;

both dereference a null pointer. That is undefined behaviour according to C standards.

The reported Misra violation is therefore completely correct.

The circumstances in which such code would be acceptable (e.g. it would be possible to write a justification for an exemption from the Misra rule, and get that approved in context of the system development) are very limited in practice.

Peter
  • 35,646
  • 4
  • 32
  • 74
  • I think you are right as I explained the software developer about the issues and then he also agreed with the things that you have mentioned. Also, I do not understand why people down voted the post. I did do research about Null pointer and MISRA rules. – Ankit Shah Mar 14 '18 at 11:58
0

You affect 0u (so, 0, so NULL) to diagBuf, and then you use it with "diagBuf[0]".

Either allocate it (malloc), or correct the declaration to fit your need (tm_uint8 diagBuf[3]; at minimum)

Tom's
  • 2,448
  • 10
  • 22
0

C11 6.3.2.3 states:

An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant

This is the only case in the C language where you can assign a pointer to the value 0 - to get a null pointer. The line in the question can never be used to set a pointer to point at address zero, since assignment of integer values to pointers is invalid C in all other cases.

If we go through the rules of simple assignment C11 6.5.16.1, then there exists no case where the left operand of = is a pointer and the right operand is an arithmetic type. This rule is very clear in the standard, code such as int* ptr = 1234;is simply invalid C and has always been invalid C (it is a "constraint violation" of the standard). Compilers letting it through without a warning/error are garbage and not standard conforming.

Simple assignment lists one valid exception (C11 6.5.16.1):

  • the left operand is an atomic, qualified, or unqualified pointer, and the right is a null pointer constant

This is the only reason why the code in the question compiles.

Now if you actually wish to point at the hardware address 0, you must write something like this:

volatile uint8_t* ptr = (volatile uint8_t*)0u;

This forces a conversion from the integer 0 into a pointer pointing at address 0. Since the right operand is neither a 0 nor a zero cast to void*, it is not a null pointer constant, and thus ptr is not a null pointer.

The C standard is clear and MISRA-C is perfectly compatible with the standard.


Bugs and issues unrelated to the question:

  • Not using volatile when pointing at a hardware address is always a bug.
  • Using diagBuf[0] + 0x40u is a bad way of setting a bit to 1. If the bit is already set, then you get a massive bug. Use | instead.
  • Assuming diagBuf is a pointer to byte, then diagBuf[0] = diagBuf[0] + 0x40u; is a MISRA-C:2012 violation of rule 10.3 since you assign a wider type ("essentially unsigned") to a narrower type. MISRA compliant code is:

    diagBuf[0u] = (tm_uint8)(diagBuf[0u] + 0x40u;);
    
Lundin
  • 195,001
  • 40
  • 254
  • 396