MISRA04 casting a typedef struct pointer as int pointer

Question

I am writing some firmware for an MSP430F5438A. I would like this code to be mostly MISRA04 complaint (I am using C99, and not C90). I am using IAR 5.51 which can check for MISRA compliance.

I have the following data structure:

typedef struct
{
    struct
    {
        uint16_t baud_rate;
        uint8_t data_bits;
        uint8_t parity;
        uint8_t stop_bits;
        uint8_t b_flow_control;
    } serial_settings;
    ...
} config_settings_t;

I want to create an instance of this structure in flash memory that can be read globally. I have separate methods already for writing to the flash.

Here is the definition of a global pointer to this structure:

volatile config_settings_t *gp_app_config = (uint8_t) 0x1800u;

This works fine and seems to be MISRA compliant.

Now, I have a set of functions that I have implemented in my flash driver that write to and read from arbitrary segments in the flash. They all take uint8_t pointers as arguments.

How can I call a function like this?

flash_segment_erase(uint8_t * p_flash, uint16_t len);

This:

flash_erase_check((uint8_t*)gp_app_config, sizeof(config_settings_t));

compiles and works fine, but is taboo according to MISRA04...

Error[Pm141]: a cast should not be performed between a pointer to object type and a different pointer to object type, this casts from type "config_settings_t volatile *" to "uint8_t *" (MISRA C 2004 rule 11.4)

Thanks, Nick

`(uint8_t) 0x1800u;` is a typo and it's actually `(uint8_t*) 0x1800u;`, isn't it? As it stands, you're initialising it with a null pointer constant. — Daniel Fischer, May 02 '13 at 20:08
No, not a typo. I am initializing with a null pointer constant... sort of. The flash can already have a value. I am just trying to say there is a structure of this form config_settings_t at address 0x1800. 0x1800 is a 16 bit pointer, but I am pointing directly to the first 8 bits of the structure. — Nick, May 02 '13 at 20:16
Oh, I see what you are saying about using a (uint8_t*) cast... both seem to do effectively the same thing, but the latter suggestion isn't MISRA complaint, whereas the original is. — Nick, May 02 '13 at 20:18
casting to a void* might have helped, but it exposed another issue... this cast will remove the volatile... ARG. — Nick, May 02 '13 at 20:22
I gathered that the address is `0x1800`, but `(uint8_t) 0x1800` is 0, so as it stands, you have `volatile config_settings_t *gp_app_config = 0;`. MISRA disallowing casts between pointers to different object types, I think you cannot escape. How bad would violating MISRA there be? — Daniel Fischer, May 02 '13 at 20:30
Not a big deal... its not a requirement by any means, I just view it as a good guide line and self checking tool. MISRA mandates that you use C90 and forbids the use of dynamic memory, but I am using C99 and I do carefully dynamically allocate memory on the heap in a few places. — Nick, May 02 '13 at 20:39
Yeah, you're right, casting it as uint8_t is truncating the address to 00. Prior to this I was doing the cast as (config_settings_t*) which is the most straight fotward, but then casting the pointer as a uint8_t for the function calls (since that's how I was using it)... it's better to violate MISRA and have code that makes sense rather than do some crazy hacking for the sake of compliance that will confuse me later. — Nick, May 02 '13 at 20:49
@DanielFischer The mandatory rule 11.2 allows casts between pointers and void pointers (but disallow casts between pointers to different types). The advisory rule 11.4 does not allow casts between pointers to different objects at all, but since it is an advisory (and rather silly) rule, you can deviate from it without raising a formal MISRA-C deviation. — Lundin, May 03 '13 at 14:33
Also, advisory rule 11.3 disallows casts between pointers and integers, which is incredibly stupid in embedded systems programming. Just ignore stupid rules, especially if they are advisory and you need not raise a formal deviation. — Lundin, May 03 '13 at 14:39

Lundin · Accepted Answer · 2013-05-03T14:50:02.207

Consider using MISRA-C:2012, since it supports C99. I have no idea if IAR supports it yet, MISRA-C:2012 was released earlier this spring.

For MISRA-C:2004, there are several things to consider.

1)

Declaring a global variable is on the border of MISRA-C compliance. There are two rules, 8.10 and 8.11, enforcing file scope variables to be static "unless external linkage is required". Whether it is required or not in your case is a bit subjective. There is no obvious reason why you need that pointer to be global.
It is strange that you declare a pointer to flash memory as a read-write pointer. That doesn't make any sense.
It doesn't make sense to cast the address into a pointer to uint8_t, while you actually want a const volatile pointer to config_settings_t. Also, casting away const or volatile keywords is banned by rule 11.5.

Therefore, I would consider to declare it as static inside the module using it, and make it read-only. Consider rewriting the code like this:

// something.h

const volatile config_settings_t* get_app_config (void); 
// use a getter function instead of a global variable

// something.c

static const volatile config_settings_t *
  gp_app_config = (const volatile config_settings_t*) 0x1800u;


const volatile config_settings_t* get_app_config (void)
{
  return gp_app_config;
}

Also consider storing the actual pointer itself in ROM (and yeah it will make that declaration even more "evil" to read...):

static const volatile config_settings_t * const
  gp_app_config = (const volatile config_settings_t*) 0x1800u;

2)

How can I call a function like this?

flash_segment_erase(uint8_t * p_flash, uint16_t len);

You shouldn't. First of all, this is part of a NVM programming driver, so it will always deal with read-only variables. Declaring them as volatile may not be necessary, but on some compilers it can save you from optimizer mishaps.

C allows all manners of wild type casts. The main problem with your code was that you casted away const and volatile.

Also, you are going to program generic data into the flash. Your flash programming driver may work on byte level, but the interface need not be uint8_t* just because of that. Changing it to void* will save the day, and save you from MISRA rule 11.2.

You should rather declare the function as:

void flash_segment_erase (const volatile void* p_flash, uint16_t len);

So now you have a const volatile config_settings_t* and you want to pass it into a function taking a generic const volatile void* parameter. That should be fully MISRA compatible.

3)

Please note that sizeof yields a variable of type size_t, which is not necessarily fully compatible with uint16_t. If size_t is for example equivalent to uint32_t, you'll break various MISRA rules. Therefore, cast to the intended type of the expression:

flash_erase_check (gp_app_config, (uint16_t)sizeof(config_settings_t));

Edit: there are various plain stupid rules such as 11.3 and 11.4 that don't allow casts like these, nor casts between integer and pointers. Since those rules would effectively ban CPU hardware registers, NVM programming, boot loaders, interrupt vector tables from MISRA-C, one has to ignore them. Otherwise, MISRA-C cannot be used in the real world.

Obviously, these rules are the result of some desktop programmer or tool vendor, without any embedded programming experience, having far too much influence over the committee (cough-ldra-cough). Despite many MISRA user complaints, the same dumb advisory rules remain in MISRA-C:2012.

I would not use `const` on a section of flash that can be erased by the program. — Doug Currie, May 03 '13 at 14:58
Making the section of flash const does present a problem because it makes the dummy write to erase the flash, as well as write operations to the flash, impossible. But the pointer on the other hand can be constant because that address is never going to change. — Nick, May 03 '13 at 18:22
In this instance the flash doesn't necessarily need to be volatile either. The main reason for making it such is because this data can be read and accessed by multiple tasks because it contains user defined settings to configure how those tasks operate. — Nick, May 03 '13 at 18:27
going back to my flash driver and making the data type for the flash pointer volatile void made everything work smoothly. Inside of those methods I am now creating temporary pointers of type uint8_t that I can compare against and reassign. Thanks for all of the advice Lundin! — Nick, May 03 '13 at 19:33
There *is* a sound rationale for those rules, although the downgrade to Advisory does allow them to be safely ignored when they are needed... and your reference to a certain tool-vendor is unfair, since two of the three now-LDRA peeps are ex-realworld embedded programmers. And the rest of us are pretty real-world too :) — Andrew, May 23 '13 at 18:27
As for the rest, I agree with your answer, and you still get a +1 :-) — Andrew, May 23 '13 at 18:28
@Andrew A sound rationale for not using CPU hardware registers or interrupt vector tables in embedded systems? Okay... Regarding a certain tool vendor, I have no idea about their particular competence, but 3 people from the same tool company and none(?) from their competition certainly stands out. Particularly since their tools are of poor quality, failing badly at listing a relevant and correct list of errors for MISRA-C:2004 compliance (even after the most buggy features are manually disabled)... — Lundin, May 23 '13 at 21:59
We have a representative from PRQA - and any of the other companies are quite welcome to contribute people, time and money. But they chose not to. — Andrew, May 23 '13 at 22:42
@Andrew Still, I have yet to hear a sane rationale for the mentioned rules, such as 11.3. How does the committee propose that a hardware register should be declared in a MISRA-compatible way? The standard way `#define REG (*(volatile uint8_t*)0x1234u)` is banned by 2004:11.3 Should MISRA-C not be used in systems where there is hardware present? Does MISRA-C encourage non-standard language extensions, such as an `@` operator? Rule 1.1 seems to disagree. How then, can we declare a hardware register without breaking either 1.1 or 11.3? — Lundin, May 24 '13 at 06:17
@Lundin - this question shows an example for the rationale behind the rule. It is unfortunate that hardware abstraction requires a similar mechanism. That is one of the reasons why the rule was downgraded to "Advisory" for C:2012. But even for C:2004 I've always put in place a standing deviation for hardware register access - which enforces a hardware abstraction layer. The rule is valid in MOST instances. HTH Andrew — Andrew, May 24 '13 at 06:24

MISRA04 casting a typedef struct pointer as int pointer

1 Answers1