MISRA demand a single point of exit for a function for a "lookup table" function

Question

Misra standard demand a single point of exit for a function, but I have the following "conversion" code

typedef enum { CASE_A, CASE_B, CASE_C } my_enum_t;

int my_conv_funct(my_enum_t value)
{
    switch(value)
    {
         case CASE_A:
             return 0;
         case CASE_B:
             return 1;
         case CASE_C:
             return 2;
         default:
             break;
    }
    log_error("ERROR!!!!!");
    assert(1==0);
}

Is this valid? I need to convert it to a single return function? And what is the best way of handling the default case?

this creates an unreachable code in theory (the error is to warn in case one add a value in the enum and not add a corresponding case)

This is an embedded system btw having those asserts create issues?

Thanks, Nick

EDITED:

the default case should be never called if there are no errors (for example a programmer add another value in the enum and doesn't add a corresponding case

another options would be to remove the default at all but that violates another misra rule

typedef enum { CASE_A, CASE_B, CASE_C } my_enum_t;

int my_conv_funct(my_enum_t value)
{
    switch(value)
    {
         case CASE_A:
             return 0;
         case CASE_B:
             return 1;
         case CASE_C:
             return 2;
    }
    //should never reach this line
    assert(1==0);
}

This will generate a warning if I compile and don't specify all the cases in the enum (I think)

Answer 1

The updated question has now been extended to include:

the default case should be never called if there are no errors (for example a programmer add another value in the enum and doesn't add a corresponding case

another options would be to remove the default at all but that violates another misra rule

I disagree...

The default is there as an error trapping mechanism - especially in real-time/embedded systems, data values may change unexpectedly (cosmic rays anyone) and it is a brave real-world engineer that does not protect against the unexpected.

How often has a default or else clause containing a comment /* Can never reach here */ actually been reached?

Answer 2

The rationale for the MISRA C "single exit" Rule is because it is a requirement for the functional safety standards (eg IEC 61508 and ISO 26262).

It is also Advisory so can be disapplied if the circumstances so require.

My personal view is that a switch statement seldom needs multiple exits - it is easy to structure to avoid them... however there are situations (eg parameter validation) where it may make sense.

--

As an aside, use of assert() is non-compliant with MISRA C as it expands to abort() which is in breach of Rule 21.8 - this is also a very undesirable behaviour in an embedded system (and questionable in a hosted environment)...

-- See profile for affiliation.

Answer 3

First of all please check this answer: Best practice for compute the function return value. The MISRA-C rule is advisory and I recommend to make a permanent deviation against it. Personally I replace it with a rule such as:

"Multiple return statements in a function should be avoided unless they make the code more readable/maintainable."

The rationale to avoid returning from multiple places inside nested, complex code is sound, but far less so in clean and readable functions.

In your specific case though, I would perhaps have rewritten the function like this (MISRA compliant without ignoring the rule):

uint32_t my_conv_funct (my_enum_t value)
{
  uint32_t result;

  switch(value)
  {
    case CASE_A: result = 0; break;
    case CASE_B: result = 1; break;
    case CASE_C: result = 2; break;
    default:
    {
      // error handling here
    }
  }
  return result;
}

Alternatively (deviating from the rule):

uint32_t my_conv_funct (my_enum_t value)
{
  static const uint32_t lut[] = { CASE_A, CASE_B, CASE_C };

  for(size_t i=0; i<sizeof lut/sizeof *lut; i++)
  {
    if(lut[i] == value)
    {
      return i;
    }
  }

  /* error handling */

  return some_error_code;
}

This assuming that the amount of items isn't large, in which case a binary search might be more inefficient.

This in turn assuming that the enum constants don't correspond to 0, 1 and 2 in which case the whole function is nonsense.

Answer 4

Is this valid?

It does not comply with the MISRA rule you described.

I need to convert it to a single return function?

To comply with the MISRA rule, yes.

And what is the best way of handling the default case?

We cannot judge what is "best" for your particular circumstances and use.

This is an embedded system btw having those asserts create issues?

The idea of an assertion is that it helps you find programming errors during development, but (in principle) it gets disabled via build options in code that is intended to be used in production. If that model is followed then the assertion itself probably does not create an issue, but the fact that the function does not return a value in the default case (if assertions are disabled) does. If the program must terminate in the event that the default case is exercised then it should call abort(), or some other function having that effect. Otherwise, it should return a sensible value in the default case.

I would probably write the function more like this:

int my_conv_funct(my_enum_t value)
{
    switch(value)
    {
         case CASE_A:
         case CASE_B:
         case CASE_C:
             break;
         default:
             log_error("ERROR!!!!!");
             assert(0);
             break;
    }
    return value;
}

There is now just one exit point from the function, and if it returns at all then it returns its argument (implicitly converted to type int).

Answer 5

Very simply:

int my_conv_funct(my_enum_t value)
{
    int result = -1;
    switch(value)
    {
         case CASE_A:
             result = 0;
             break;
         case CASE_B:
             result = 1;
             break;
         case CASE_C:
             result = 2;
             break;
         default:
             break;
    }
    if(result == -1)
    {
         log_error("ERROR!!!!!");
         assert(1==0);
    }
    return result;
}