5

I have a project I'm developing in C99 and I'm trying to make it compliant with the MISRA 2012 standard.

In one file I define an enum where each value should be treated as a flag:

/**
 * Enumerates the configurable options for performing calibration.
 */
typedef enum
{
  CALIBRATION_DEFAULT_OPTIONS=0,  /**< Calibrate with default options */
  CALIBRATION_RESET_POSITION=1,   /**< Ensure window is fully open and motor re-homed */
  CALIBRATION_FORCE_RECALIBRATE=2 /**< Force recalibration even if calibration data exists */
} CALIBRATION_OPTIONS_T;

I'd like to be able to declare something like:

CALIBRATION_OPTIONS_T options = CALIBRATION_RESET_POSITION | CALIBRATION_FORCE_RECALIBRATE;

I also define one function that accepts a CALIBRATION_OPTIONS_T parameter and performs different logic depending on which flags are set:

// If forced to recalibrate, do so regardless of whether metrics exist in
// EEPROM or not.
if ((options & CALIBRATION_FORCE_RECALIBRATE) != 0U)
{
  MOTION_ResetCalibrationData();
  calibration = performCalibrationRoutine();
}

// Otherwise try fetching existing metrics from EEPROM. If they exist, return
// these metrics.
else if (tryFetchStoredMetrics(&calibration))
{
  if ((options & CALIBRATION_RESET_POSITION) != 0U)
  {
    calibration.lastPosition = 0;
    resetMotorPosition();
    storeMetrics(calibration);
  }
}

However, when I lint my project with PC-lint Plus I get the following output explaining that this code violates MISRA 2012 Rule 10.1:

  if ((options & CALIBRATION_FORCE_RECALIBRATE) != 0U)
       ~~~~~~~ ^
*** LINT: src\c\motionCalibrator.c(645) note 9027: an enum value is not an appropriate left operand to & [MISRA 2012 Rule 10.1, required]

  if ((options & CALIBRATION_FORCE_RECALIBRATE) != 0U)
               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*** LINT: src\c\motionCalibrator.c(645) note 9027: an enum value is not an appropriate right operand to & [MISRA 2012 Rule 10.1, required]

  if ((options & CALIBRATION_FORCE_RECALIBRATE) != 0U)
       ^
*** LINT: src\c\motionCalibrator.c(645) warning 641: implicit conversion of enum 'CALIBRATION_OPTIONS_T' to integral type 'unsigned int'

    if ((options & CALIBRATION_RESET_POSITION) != 0U)
         ~~~~~~~ ^
*** LINT: src\c\motionCalibrator.c(655) note 9027: an enum value is not an appropriate left operand to & [MISRA 2012 Rule 10.1, required]

    if ((options & CALIBRATION_RESET_POSITION) != 0U)
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
*** LINT: src\c\motionCalibrator.c(655) note 9027: an enum value is not an appropriate right operand to & [MISRA 2012 Rule 10.1, required]

    if ((options & CALIBRATION_RESET_POSITION) != 0U)
         ^
*** LINT: src\c\motionCalibrator.c(655) warning 641: implicit conversion of enum 'CALIBRATION_OPTIONS_T' to integral type 'unsigned int'

In particular, the MISRA 2012 standard advises against using & with enums for these two reasons:

  1. An operand of essentially enum type should not be used in an arithmetic operation because an enum object uses an implementation-defined integer type. An operation involving an enum object may therefore yield a result with an unexpected type. Note that an enumeration constant from an anonymous enum has essentially signed type.

  2. Shift and bitwise operations should only be performed on operands of essentially unsigned type. The numeric value resulting from their use on essentially signed types is implementation-defined.

I'd like to know if there's a MISRA-compliant way I can use flag-like enums and test that specific flags are set.

Tagc
  • 8,736
  • 7
  • 61
  • 114
  • 3
    Use defines instead for flags. Enums are not meant for bit flags. – Fredrik Aug 16 '18 at 08:15
  • @Fredrik Doesn't using defines reduce type safety though? For example given something like this (https://hastebin.com/uyuzoyowab.c), the compiler helpfully generates a warning about the first assignment ("warning: #188-D: enumerated type mixed with another type") but nothing about the second. – Tagc Aug 16 '18 at 08:24
  • Oh, I work mainly in C# where enums can be used to represent bit flags with full type safety (https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/enumeration-types#enumeration-types-as-bit-flags). Shame if C doesn't support this. – Tagc Aug 16 '18 at 08:25
  • @9769953 0, 1, 2 are exclusive. Each corresponds to one bit being set, with 0 to treat as "absence of flag". I'd continue that pattern as 4, 8, etc. if I added more options in the future. – Tagc Aug 16 '18 at 08:26
  • @Tagc i don't see how enums give any type safety anyhow. And when using defines make sure your literals are unsigned and not signed. – Fredrik Aug 16 '18 at 08:27
  • I guess it's a pick your poison deal then. Either I use #defines and reduce type safety, allowing me to write buggy code like `CALIBRATION_OPTIONS_T options = CALIBRATION_RESET_POSITION | 8;` without getting any warnings from the compiler; or else I use enum flags like in C# but deviate from MISRA guidelines. – Tagc Aug 16 '18 at 08:33
  • 3
    @Tagc If you feel that you really want to use enums with MISRA it would probably work if you cast them to an unsigned type before doing arithmetic on them, I would stick with defines though. – Fredrik Aug 16 '18 at 08:33
  • 2
    @Tagc Just don't write such buggy code and you are fine ;) – Fredrik Aug 16 '18 at 08:34
  • I might switch to bitflags during a later refactor. However, following @Fredrik's suggestion and casting to unsigned types works nicely though: `if (((uint16_t)options & (uint16_t)CALIBRATION_FORCE_RECALIBRATE) != 0U)`. This way I can declare `options` with more type-safety than defines would give me, and I can guarantee the behaviour of `&` in a way that makes MISRA happy. – Tagc Aug 16 '18 at 08:38
  • 1
    @Tagc actually you threw typesafety out of the window with the cast. Because it says: ignore the type it has and make it what I want – Kami Kaze Aug 16 '18 at 08:41
  • @KamiKaze In the function body itself I'm forced to use casts, sure, if I want to use enums instead of flags. But using enums means that I get better type safety in the rest of my codebase - the compiler will warn me if I wrongly declare the `options` parameter I pass to this function, so I can be reasonably sure the function receives valid arguments from any call within my codebase. It's not ideal, but I guess C is low-level and just doesn't have a sufficiently powerful type system for this sort of thing. Any option has trade-offs. – Tagc Aug 16 '18 at 08:46
  • @9769953 The Linux kernel is a horrible example for how to write canonical C code. Don't even think of looking at it as a reference for how to do proper C programming. The Linux kernel coding standard looks like an amateur garage project compared to serious coding standards like MISRA-C. – Lundin Aug 16 '18 at 08:48
  • @Tagc unless you're compiling parts of your code as C++, the compiler is highly unlikely to warn you about anything. Enums are IMO better style than defines in C, but they aren't type safe at all (they create a new type which is 100% "compatible" with a builtin type; conversions are transparent, and the enumerators themselves don't even have that type). Don't confuse "safety" with "MISRA compliance", the two are pretty much orthogonal. – Alex Celeste Aug 16 '18 at 08:53
  • 1
    What I mean by "type safety" is that my compiler (ARM C compiler v5.06) *does* actually warn me if use enums and do something silly like `options = CALIBRATION_RESET_POSITION | 8`. It still compiles, but a warning is better than nothing. Whereas using defines, the compiler doesn't even issue a warning. However, I'm swayed and I've switched to using defines now. I'll just try to make sure I don't do anything silly like `CALIBRATION_RESET_POSITION | 8`. – Tagc Aug 16 '18 at 08:58

1 Answers1

10

This boils down to the essential type model and rule 10.1. You are only allowed to do bitwise operations on types that are essentially unsigned. Enums are treated as their own unique type by MISRA-C.

Doing things like CALIBRATION_OPTIONS_T options = CALIBRATION_RESET_POSITION | CALIBRATION_FORCE_RECALIBRATE; is otherwise fine and pretty canonical C, but you have to resort to using unsigned constants. To take type safety a bit to the extreme, you could do this:

typedef uint32_t CALIBRATION_OPTIONS_T;
#define CALIBRATION_DEFAULT_OPTIONS   ((CALIBRATION_OPTIONS_T)0x00u) /**< Calibrate with default options */
#define CALIBRATION_RESET_POSITION    ((CALIBRATION_OPTIONS_T)0x01u) /**< Ensure window is fully open and motor re-homed */
#define CALIBRATION_FORCE_RECALIBRATE ((CALIBRATION_OPTIONS_T)0x02u) /**< Force recalibration even if calibration data exists */

where the hex notation is self-documentating code showing that these are bit masks, the u suffix is required by MISRA in some circumstances and the uint32_t is there to block potential implicit type promotions.

Please note that using enums don't necessarily give increased type safety, but rather the opposite. They are in many cases treated like plain int, in other cases as implementation-defined size integers. Their type safety is pretty much broken by C language design. though you can make them safe with some tricks, see my posts at How to create type safe enums?.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 1
    Hm, yeah you've convinced me. I'll switch to that approach instead of enum flags. – Tagc Aug 16 '18 at 08:49
  • 1
    @Tagc Also please note that this isn't just some false positives; the MISRA rule is a very sound one. Enums are fairly non-portable, particularly so in for example small microcontroller systems, and often gives you a signed result, which is dangerous to use together with bitwise operations. – Lundin Aug 16 '18 at 08:52