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: A Focus on Writing Clear, Maintainable Code
Colin Walls

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.

M↓ MARKDOWN HELP
italicssurround text with
*asterisks*
boldsurround text with
**two asterisks**
hyperlink
[hyperlink](https://example.com)
or just a bare URL
code
surround text with
`backticks`
strikethroughsurround text with
~~two tilde characters~~
quote
prefix with
>

nathancharlesjones
Score: 1 | 7 months ago | 1 reply

@MaxP, that's a great tip about const, to go along with the advice about static!
Another one I heard (related to Rule 16.4 about having a default statement inside every switch statement) is to always include an else statement with any if 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.

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

I have never heard of a "compulsory else" rule, but it does make some sense.

SimonSmith
Score: 1 | 7 months ago | 2 replies

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!

ColinWallsSpeaker
Score: 0 | 7 months ago | 2 replies

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.

yen
Score: 0 | 7 months ago | 2 replies

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
};

Andre.Hartmann
Score: 2 | 7 months ago | 1 reply

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 ofval`, 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.

yen
Score: 0 | 7 months ago | no reply

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.

ColinWallsSpeaker
Score: 1 | 7 months ago | no reply

In a word: yes.

SimonSmith
Score: 0 | 7 months ago | 1 reply

Yes, we know it may not be the most portable implementation, but it works well and we don't intend to change the toolchain.

ColinWallsSpeaker
Score: 1 | 7 months ago | no reply

That is absolutely fine, so long as it is fully documented.

Robert Hancock
Score: 0 | 7 months ago | no reply

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.

yen
Score: 0 | 7 months ago | 1 reply

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.

ColinWallsSpeaker
Score: 0 | 7 months ago | 1 reply

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.

yen
Score: 0 | 7 months ago | 1 reply

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!

ColinWallsSpeaker
Score: 1 | 7 months ago | 1 reply

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.

yen
Score: 0 | 7 months ago | no reply

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.

Insta_Jun
Score: 0 | 7 months ago | 1 reply

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.

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

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.

Nathan3
Score: 1 | 7 months ago | 1 reply

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.

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

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.

DrK
Score: 0 | 7 months ago | 1 reply

rule 1.2 : Is there few common examples that illustrate this rule?

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

A number of compilers offer extensions as additional keywords. Examples include interrupt, packed and unpacked.

Andre.Hartmann
Score: 0 | 7 months ago | 1 reply

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.

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

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.

JGrunklee
Score: 1 | 7 months ago | 1 reply

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?

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

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

SimonSmith
Score: 0 | 7 months ago | 1 reply

Thanks Colin for a good talk.

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

Thank you for the feedback.

Carlos.Amaya
Score: 0 | 7 months ago | 1 reply

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!

ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

Thank you for the feedback.

MaxP
Score: 1 | 7 months ago | 1 reply

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;
}
ColinWallsSpeaker
Score: 0 | 7 months ago | no reply

That is a very good suggestion. Thanks.

OUR SPONSORS

OUR PARTNERS