Home > On-Demand Archives > Talks >
MISRA C: A Focus on Writing Clear, Maintainable Code
Colin Walls - Watch Now - EOC 2024 - Duration: 28:11
MISRA C is a programming standard that is focussed on writing safe, secure code using the C language. This is achieved by the definition of a number of guidelines that lead the developer away from operations and constructs that may compromise the safety of the code. By taking a fresh look at the standard, this session concentrates on the benefits of MISRA C in an additional priority in embedded software development: the writing of clear and maintainable code.
I have never heard of a "compulsory else" rule, but it does make some sense.
Re: question about unions in Q&A. We use this pattern below a lot with struct inside union, where we can set all bits easily via "value = 0u":
typedef union
{
uint32_t value;
struct
{
uint32_t bitX : 1;
uint32_t bitY : 1;
// etc
}
} myTypeName_t;
Another example with union inside struct where we can access the data either as bits in a structure, or as a byte array when transmitting/receiving it without needing any conversion functions:-
typedef struct
{
union
{
type_t value;
char bytes[sizeof(type_t)];
}
uint32_t crc;
}
} myTypeName_t;
Sorry about the formatting, can't get Markdown to work!
I understand the attraction of using unions in this way, but it has 2 implications. First, it is compiler dependent; the C language standard does not dictate how a structure is laid out in memory, so mapping onto another data type is "dangerous". Of course, you can do this if you carefully document the dependency. Also, it hides the fact that there is a read/OR/write process, which I would argue is making the code slightly less readable/clear.
Hey Colin,
So even if the union members all have the same size individually, writing one member and reading back the other can be problematic because it is up to the compiler to decide the layout?
typedef union
{
uint32_t val;
uint8_t byteArray[4];
//byteArray[0] might be any byte of val, all dependent on the compiler implementation?
struct bitField32
{
uint32_t bit0 : 1;
//...
uint32_t bit31 : 1;
}bits;
//similarly, bits.bit0 might be any bit within val
};
What you need to be aware of, is that a union containing uint32_t
and uint8_t [4]
is not portable between big- and little endian platforms. byteArray[0] might not be any byte of
val`, but should either be the highest or lowest byte.
Despite that, I have used unions with uint32_t
and the corresponding bitfields many times and at least with my compilers it always worked (e.g. using the same code on PC and microcontroller for data exchange). Your mileage may vary.
Thanks for the info! Yeah I understand depending on endianness byteArray[0] could be MSB or LSB. I was also thinking since it is up to the compiler they can potentially do something unconventional (especially the non-standard ones), like making byteArray[0] be byte 1 or 2 of val.
In a word: yes.
Yes, we know it may not be the most portable implementation, but it works well and we don't intend to change the toolchain.
That is absolutely fine, so long as it is fully documented.
One thing to be aware with that type of union usage is that although C allows it as implementation-defined behavior, in C++ it is explicitly undefined behavior to access a union other than through the data member most recently written to.
Thanks again for the feedback Colin! One way I have seen unions being used would be as an event parameter for communication among tasks/threads.
Depending on the event type, different members of the event parameters are accessed (sorry about the formatting):
struct eventDetails
{
event_enum eventType;
union eventParams
{
uint32_t paramForEventA;
uint8_t paramForEventB;
//other params
};
};
Would love to know your thoughts on this.
As I have commented in another response here, structure [and union] layout is compiler-dependent and that can lead to issues down the line. Even if the code works now, a change of compiler in future might cause serious problems.
Hey Colin,
Can you elaborate on this please? I thought even though the layout is compiler-dependent, as long as the access is fine (write/read to same member), changing the compilers shouldn't cause problems right?
Using the above code snippet as example, say task 1 sends the event detail to task 2. If the eventType is EventA, task 1 sets the value needed for paramForEventA (i.e., details.params.paramForEventA = someValue). When task 2 gets notified of the event, it first checks the eventType and knows to access paramForEventA (i.e., receivedVal = details.param.paramForEventA). Would implementations like this be problematic if a change of compiler is needed?
Thanks again for your time!
I think you would get away with it in this example, as [if I understand it correctly] there is a union of a 1-byte data item with a 4-byte one and it doesn't matter which byte of the 4-byte one is shared.
Ah I think I understand the comment about compiler-dependent better. Thanks for clarifying! I do have a follow-up though, but will reply under the corresponding thread.
Hello Collin, what is a good way to learn MISRA standard so that we can implement this guideline in our daily coding style? Is that suggestible for you that we just look through the whole directives and rules of the MISRA? Or do you have any other suggestions from your experience? My personal learning practice of the MISRA or other guidelines like CERT C is read one or two rules every day.
I think that the approach you suggest makes sense. Running code through a MISRA checker and studying the failures is another way of gaining insight. I am sure that someone, somewhere offers training.
Following and the Q&A question about unions: how would you manage a message structure that can contain different types of data (some simple types, other being structures) ?
I tend to use a struct with a type enum and a data union. I am interesting to see how this topic can be tackled without union.
I cannot give a definitive answer without seeing the code/data in more detail. However, broadly speaking, if you have a bunch of bits, which arrives most likely as a sequence of 32-bit words, and you need to extract field from them, it is far better to code the "bit bashing" yourself than rely on structure/union layouts that are compiler dependent.
rule 1.2 : Is there few common examples that illustrate this rule?
A number of compilers offer extensions as additional keywords. Examples include interrupt, packed and unpacked.
Thanks Colin, that was (again) a great talk!
I have one addition regarding rule 5.3: that is not only about global and local variables (which could be distinguished by naming schemes), but also local variables within different scopes:
void bar() {
int foo = 0;
if (foo) {
int foo = 1;
}
}
I even think I saw cppcheck complaining about such shadowing, but I was not able to reproduce that now.
That is correct. The rule actually refers to "inner scope" and "outer scope", which I guess can mean global and local, but can also refer to nested blocks.
Thanks Colin! Regarding Rule 17.8 ("don't modify function parameters") - it is common practice to use pointer types as "output" parameters. Do you/MISRA consider this practice maintainable? On a related note, how does MISRA C feel about side-effects in general?
An interesting question. For the most part, MISRA C warns against the intentional use of side-effects. However, this leaves open the question of how a function can return more than one data item. Use of pointer parameters is an obvious way to achieve this and I believe that it is MISRA compliant. Alternatives would be to return a structure or a pointer to a static data item [structure or array].
Thanks Colin for a good talk.
Thank you for the feedback.
Thank you Colin for this concise and informative presentation. I really liked the examples you used to clarify the rules by MISRA. Thank you for sharing!
Thank you for the feedback.
Thanks Colin, very informative overview of the MISRA C. I'd like to add that to better enforce 17.8 it is possible to declare parameters as "const":
int times2( int n ); // prototype stays the same
int times( int const n ) // n is const int
{
n *= 2; // <- error since n is const
return n;
}
That is a very good suggestion. Thanks.
@MaxP, that's a great tip about
const
, to go along with the advice aboutstatic
!Another one I heard (related to Rule 16.4 about having a
default
statement inside everyswitch
statement) is to always include anelse
statement with anyif
statement. My thoughts on that are mixed, but I feel like if you buy the MISRA rule, then that also must follow by the same logic.