Domanda

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

È stato utile?

Soluzione

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.

Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top