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.
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).