thoughts on unions in C, with regards to MISRA

Question

Misra says to ban all unions. I also know that deviations are allowed as long as they are discussed and documented thoroughly.

We have a microcontroller and an external eeprom to store statistical data (event/error logging, parameter settings and whatnot).

The eventlog consists of around 80+ event counters some being 8, 16 and 32 bits (all unsigned). The parameter storage consists of around 200 parameters, also mixed with 8, 16 and 32 bit values (unsigned).

We are rewriting all code to be MISRA compliant and as these values were previously defined is as follows:

typedef struct
{
  U16BIT eventLogVar1;
  U32BIT eventLogVar2;
  U8BIT  eventLogVar3;
  U8BIT  eventLogVar4;
  U32BIT eventLogVar5;
} EVENT_LOG;

typedef union
{
  EVENT_LOG log;
  U8BIT     array[sizeof(EVENT_LOG)];
} ELOG;

ELOG log;

Now this is not really MISRA compliant. Same goes for the parameter log. However this is the most easiest way to read and write from the eeprom, because I just have to read/write through the array to read/write from the eeprom.

We have a few other rules that we simply are not allowed to break. No global (extern) variables (through header files). All local variables, if needed should only be accessed through get/set functions.

That implies that if we need to fully write out all these parameters that these each should get their own get/set functions for altering them throughout the application.

On of the solutions that I've thought on was the following:

#ifdef EITHER
enum
{
    eventLogVar1 = 0; /* 00 */
    pad01;            /* 01 */
    eventLogVar2;     /* 02 */
    pad03;            /* 03 */
    pad04;            /* 04 */
    pad05;            /* 05 */
    eventLogVar3;     /* 06 */
    eventLogVar4;     /* 07 */
    eventLogVar5;     /* 08 */
    pad09;            /* 09 */
    pad10;            /* 10 */
    pad11;            /* 11 */
}
#else /* OR */
#define eventLogVar1 0 /* 2 bytes */
#define eventLogVar2 2 /* 4 bytes */
#define eventLogVar3 6 /* 1 byte  */
#define eventLogVar4 7 /* 1 byte  */
#define eventLogVar5 8 /* 4 bytes */
#endif
#define eventLogLastLength 4

U8BIT eventLog[eventLogVar5 + eventLogLastLength];

U8BIT getU8BIT(U8BIT index){}
U16BIT getU16BIT(U8BIT index){}
U32BIT getU32BIT(U8BIT index){}

void setU8BIT(U8BIT index, U8BIT val){}  
void setU16BIT(U8BIT index, U16BIT val){}
void setU32BIT(U8BIT index, U32BIT val){}

However, this poses a combersome refactoring if values are added or removed. It also means that values of type array can not be used (and there are a few) which can be altered by defines in length if more or less of a certain type of, e.g., sensors, are used.

---

What are your thoughts on this specific problem. Would I/we be better off documenting our deviation from the MISRA-standard in this specific case and only use this deviation at this specific place or are there better solutions to this problem?

Answer 1

Your log union is exactly the kind of union you should be allowed to use, what it is doing is data packing, something MISRA explicitly states to be an acceptable deviation, so a deviation is what you should do. Pretty much everyone who uses MISRA deviates from this rule in this very manner. (It is a rather bad rule and it looks like it will get demoted to advisory or removed completely in the next MISRA version.)

But you will need to document:

• whether padding bytes and alignment will be an issue.
• whether endianess might be an issue, if the code needs to be portable.

To dodge the padding/alignment issue, you can write something like:

COMPILE_TIME_ASSERT( (sizeof(union_member1)+sizeof(union_member2)+...) ==
                     sizeof(union_type) );

where COMPILE_TIME_ASSERT is some macro that yields a compiler error if not passed a positive value. This ensures that no struct/union padding is present.

---

Further comments:

enum is a bad solution since it has a number of flaws of its own: a variable of the enum type has implementation-defined size, while an enumeration constant has type signed int. This will collide with the MISRA rules regarding implicit type conversions, you will be forced to add numerous typecasts.

All local variables, if needed should only be accessed through get/set functions.

They also need to be declared as static to reduce their scope. I noticed from your snippet that you don't do this. static is enforced by MISRA:2004 8.11.

Answer 2

Unions are a very useful construct, when used properly.

They also have undefined behaviour when one tries to do clever things with them. It is to prevent the undefined behaviour that the MISRA guidelines are trying to prevent. But even the Rule (18.2 in MISRA C:2004) gives cases where unions are useful.

The example you give is one such useful situation, and is exactly the sort of situation that the MISRA deviation procedure is there for.

---

Disclaimer: I am a member of the MISRA C working group, but I am posting in a personal capacity. My views should not be taken as official MISRA policy.

Answer 3

What does MISRA say about memcpy? Instead of having a union, why not just use EVENT_LOG? When you serialize to/from EEPROM use a temporary array like this:

EVENT_LOG log;

U8BIT array[sizeof(EVENT_LOG)];
// populate array

memcpy(&log, array, sizeof(EVENT_LOG));

// do similar thing when writing to eeprom

The union mixes the data and the serialization format together, where a function might be more suited?

void EVENT_LOG_write_to_eeprom(const EVENT_LOG*);
void EVENT_LOG_read_from_eeprom(EVENT_LOG*);