MISRA C++ 2008 Rule 5-2-7 violation: An object with pointer type shall not be converted to an unrelated pointer type, either directly or indirectly

Question

In the following example:

void bad_function()
{  
  char_t * ptr = 0;

  // MISRA doesn't complains here, it allows cast of char* to void* pointer
  void* p2 = ptr;

  // the following 2 MISRA violations are reported in each of the casts bellow (two per code line)
  // (1) Event misra_violation:     [Required] MISRA C++-2008 Rule 5-2-7 violation: An object with pointer type shall not be converted to an unrelated pointer type, either directly or indirectly
  // (1) Event misra_violation:     [Required] MISRA C++-2008 Rule 5-2-8 violation: An object with integer type or pointer to void type shall not be converted to an object with pointer type
  ptr = (char_t*) (p2); 
  ptr = static_cast<char_t*> (p2); 
  ptr = reinterpret_cast<char_t*> (p2); 
}

MISRA 5-2-8 and 5-2-7 violations are reported.

How I can remove this violation ?

I need someone experienced with C++ static analysis to help me. I am hitting my head with this stupid rules from few days.

According to MISRA C++ standard (MISRA-Cpp-2008.pdf: Rule 5-2-7 (required): An object with pointer type shall not be converted to an unrelated pointer type, either directly or indirectly.

Ok but we have a lot of code which for example needs to convert address to char* and then to use it with std::ifstream, which read(char* buffer, int length) function requires to type cast the address to (char_t*). So how according to MISRA guys someone can program in C++ and not using at all any casts? The standard doesn't say HOW pointer conversion then must be done.

In my production code my problems are in file reading operations using read with std::ifstream from files in predefined data structures:

if (file.read((char_t*)&info, (int32_t)sizeof(INFO)).gcount() != (int32_t)sizeof(INFO)
{
                LOG("ERROR: Couldn't read the file info header\n");
                res = GENERAL_FAILURE;
}

How is supposed to do it according to MISRA?

So are there any solutions at all?

EDIT: Peter and Q.Q. answers are both correct, it seems that MISRA really wants to do everything without any casts which is hard to be done if the project is in the final stage. Therea are two options:

1 - document the MISRA deviations one by one and explain why casts are Ok, explain how this has been tested (Q.Q. suggestion)

2 - use byte array from char type for file.read(), then after safely reading the file content cast the byte array to the headers content, this must be done for each member one by one because if you cast char* to int32_t this is again Rule 5-2-7 violation. Sometimes it is too much work.

Theurich

What is char_t? Do you have to use it? – Pacesetter

It is just typedef char char_t; MISRA doesn't like using base types. – Theurich

Why are you casting to a void*? – Lashandralashar

There's a very sound reason why MISRA doesn't like the default char type for anything but C strings, namely the implementation-defined signedness. If you are only using char for strings, you are fine. If you are using it for arithmetic, other than what's listed as valid exceptions by MISRA, creating a char_t typedef is not the correct way to go: you should then have used uint8_t or int8_t. – Locklin

Yes, sorry, I am using int8_t. – Theurich

Casting from void * to other type assumes the pointer to be properly aligned, otherwise UB. – Lidia

I am using void* - just to show that MISRA allows it. My problem in the production code is with converting pointers to data structures ;like (&header) to char* pointer which is then passed to file io functions. I am unable to find a way to do this. – Theurich

There is nothing wrong with the OPs code according to the C++ standard. Question is what to do about MISRA coding specification disallowing it. – Pyrethrum

@BajMile The key here is to read the MISRA rule and try to understand the rationale behind the rule. Because the problem is most often not with MISRA itself, but with the static analyser. If you have understood why MISRA created the rule and can deduct that the issue they were concerned about is not present in your code, you can likely just ignore the warning. I don't know the reason behind this rule so I can't help there. – Locklin

are you using an automated tool to find these guideline violations? Doesn't this tool have an instruction manual? – Selfcontent

Technically pointer to POD is very related to pointer to char, so I suggest you look at the interpretation of the rules by the code checker. – Lidia

Just a passing comment on the first comment "it allows cast of char* to void*" -- the conversion from char* to void* in the code does not use a cast. A cast is something you write in your source code to tell the compiler to do a conversion. – Hedron

Maybe trick with enum may remove Misra warning, but not sure it is better. – Finfoot

@Lundin, no uint8_t or int8_t are not valid alternatives. The strict aliasing rule gives special behavior only to narrow character types and not extended integer types. – Efrem

@BenVoigt That may be so, but any implementation which treats uint8_t differently than unsigned char when it comes to aliasing would be so utterly useless that everyone using it deserve all the bugs they can get. It doesn't matter what the standard says: just because an implementation conforms to the standard, it doesn't mean that the implementation should actively seek to be useless in real-world applications. If I encountered a compiler which gave alias-related bugs because I use uint8_t instead of char, I'd throw it out the window, the standard be damned. – Locklin

"cast the byte array to the headers content," - you really don't want to do that , so the MISRA restriction is good in this case. – Goins

Answer 1

The basic reason for the MISRA rule is that converting any pointer/address to any non-void pointer allows using that address as if it is a different object than it actually is. The compiler would complain about an implicit conversion in those cases. Using a typecast (or C++ _cast operators) essentially stops the compile complaining and - in too many circumstances to count - dereferencing that pointer gives undefined behaviour.

In other words, by forcing a type conversion, you are introducing potential undefined behaviour, and turning off all possibility of the compiler alerting you to the possibility. MISRA think that is a bad idea .... not withstanding the fact that a lot of programmers who think in terms of ease of coding think it is a good idea in some cases.

You have to realise that the philosophy of MISRA checks is less concerned about ease of programming than typical programmers, and more concerned about preventing circumstances where undefined (or implementation defined or unspecified, etc) behaviours get past all checks, and result in code "in the wild" that can cause harm.

The thing is, in your actual use case, you are relying on file.read() correctly populating the (presumably) data structure named info.

if (file.read((char_t*)&info, (int32_t)sizeof(INFO)).gcount() != (int32_t)sizeof(INFO)
{
            LOG("ERROR: Couldn't read the file info header\n");
            res = GENERAL_FAILURE;
}

What you need to do is work a bit harder to provide valid code that will pass the MISRA checker. Something like

std::streamsize size_to_read = whatever();
std::vector<char> buffer(size_to_read);  

if (file.read(&buffer[0], size_to_read) == size_to_read)
{
      //  use some rules to interpret contents of buffer (i.e. a protocol) and populate info
      // generally these rules will check that the data is in a valid form
      //   but not rely on doing any pointer type conversions
}
else
{
            LOG("ERROR: Couldn't read the file info header\n");
            res = GENERAL_FAILURE;
}

Yes, I realise it is more work than simply using a type conversion, and allowing binary saves and reads of structs. But them's the breaks. Apart from getting past the MISRA checker, this approach has other advantages if you do it right, such as the file format being completely independent of what compiler is used to build your code. Your code depends on implementation-defined quantities (the layout of members in a struct, the results of sizeof) so your code - if built with compiler A - may be unable to read a file generated by code built with compiler B. And one of the common themes of MISRA requirements is reducing or eliminated any code with behaviour that may be sensitive to implementation-defined quantities.

Note: you are also passing char_t * to std::istream::read() as first argument and an int32_t as the second argument. Both are actually incorrect. The actual arguments are of type char * and std::streamsize (which may be, but is not required to be an int32_t).

Circumferential

Peter, thanks for the great answer. I had doubts that someone reasonable has wrote this MISRA standard so now it seems that it is so. If I use byte arrays, no need by std::vector because in most cases headers sizes are small, I will have later other problems when converting the byte arrays to accual data - ints. shorts, etc. I know how to do this in the MISRA way, without any casts, but it will take too much time. I will keep in mind this. I have defined char_t as char to satisfy another MISRA rule and it doesn't complain about std::streamsize but you are generally right about std::streamsiz – Theurich

No problem. I used std::vector<char> to create a buffer for convenience. However, since std::vector does dynamic memory allocation, that may not be acceptable depending on your system requirements. Just remember that most guidelines for a "safer" C++ (or a safer <any other language>) focus on safety first, programmer convenience/productivity second - because programmer convenience/productivity often means using techniques that compromise safety. That is as true of MISRA as it is for any other comparable guidelines you might pick. – Circumferential

I could be wrong, but does MISRA C++ really force the explicit type conversion? I appreciate that some implicit type conversions fall foul of other rules, but... – Purely

@Purely MISRA C++ is a set of guidelines, and not all projects have to comply with MISRA. The ones that do, normally do so to meet some specific objectives (e.g. related to independently verifiable system safety). If you are working on such a project, then you are required to either comply, refer to an approved exemption, or justify a new exemption in context of the project. Your justification would need to convince relevant decision makers (such as a regulator who has endorsed the objectives, or senior engineers responsible for ensuring the project meets its objectives). – Circumferential

I'm fully aware of what MISRA is ;-) My profile offers a hint... – Purely

Answer 2

Converting unrelated pointers to char* is not a good practice. But, if you have a large legacy codebase doing this frequently, you can suppress rules by adding special comments.

Melodramatic

Yes everyone uses it. For example std::ifstream has read() function which needs char*. Then if I try - file.read((char_t*)(&header), (int32_t)sizeof(HEADER)) I am getting MISRA violation. I don't know about any way this to be solved even if I rewrite the whole code to use only char buffers in all file read operations like this. – Theurich

I guess you need these convertions for serialization and deserialization.So, you can add serialization methods. In the methods you can suppress "Rule 5-2-7" explaining your argument in a commnet block. – Melodramatic

Just because "everyone uses it" does not mean it is a good idea... – Purely

Answer 3

fread is a perfectly good C++ function for file input and it uses void*, which MISRA allows.

It also is good at reading binary data, unlike fstream which processes all data through localized character conversion logic (this is the "facet" on an iostream, which is configurable, but the Standard doesn't define any portable way to achieve a no-op conversion).

The C-style of fopen/fclose is unfortunate in a C++ program, since you might forget to cleanup your files. Luckily we have this std::unique_ptr which can add RAII functionality to an arbitrary pointer type. Use std::unique_ptr<FILE*, decltype(&fclose)> and have fast exception-safe binary file I/O in C++.

---

NB: A common misconception is that std::ios::binary gives binary file I/O. It does not. All it affects are newline conversion and (on some systems) end-of-file marker processing, but there is no effect on facet-driven character conversion.

Efrem

Thanks for the answer. MISRA Rule 27-0-1 doesn't allow to use the stdio.h and fopen, fread. We have local policy that also forbid that. std:ifstream must be used. I will check anyway if I can trick it using std::unique_ptr . – Theurich

Well, if stdio is forbidden, I guess you can use the OS function open/read/write/close. Same approach using std::unique_ptr and a deleter function applies. – Efrem

Doesn't MISRA-C++ also ban dynamic allocation, like MISRA-C does? If dynamic allocation is banned, then smart pointers are consequently banned too. – Locklin

@Locklin that does not follow. unique_ptr can manage resources other than memory, if you set a cleanup function which is not a deallocator – Efrem

One of the compilers used are for RTOS and doesn't support unique_ptr and C++ x11. – Theurich