MISRA C 2012 Rule 15.4 and replacing goto's with break's

Question

Regarding to MISRA C 2012 rule 15.4 - "There should be no more than one break or goto statement used to terminate any iteration statement." - is this example correct? Can anyone confirm this with some tool (MISRA checker)?

do {
    retval = do_smth();
    if (retval != OK) {
        break;
    }

    retval = do_smth2();
    if (retval != OK) {
        break;
    }

    retval = do_smth3();
} while (0u);

This is just a concept, but what I am trying here is to replace cascade of goto (unfortunately banned in this case) with cascade of break. My point is that do { } while(0u); is not an iteration statement.
What you think?

Answer 1

First of all, your code does indeed not follow rule 15.4 since you have 3 break inside an iteration statement¹⁾. But it is just an advisory one and there's nothing wrong with using multiple breaks like you do as long as the code is readable and easy to follow.

The main rationale with these MISRA rules is to prevent "compound statement spaghetti" where complex code breaks out from multiple nested compound statements. It's important to understand the rationale before following these rules blindly. So in this case just consider leaving the code as it is - no deviation is needed for advisory rules.

Otherwise, there's a few options as shown below:

---

One problem with MISRA-C is that it doesn't allow multiple returns from a function, even when it makes the code more readable. Otherwise the obvious and most readable solution would be to use a function:

type do_stuff (void);
{
  type retval;

  retval = do_smth();
  if (retval != OK) { return retval; }

  retval = do_smth2();
  if (retval != OK) { return retval; }

  retval = do_smth3();

  return retval;
}

My usual solution is to make a permanent MISRA-C deviation from the multiple return rule and allow it in cases where it makes the code more readable, like it does in this case.

Otherwise, the 2nd best option might be the old "on error goto" - the rule banning goto was relaxed in MISRA-C:2012 so it's just advisory now.

  retval = do_smth();
  if (retval != OK) { goto error; }

  retval = do_smth2();
  if (retval != OK) { goto error; }

  retval = do_smth3();
  if (retval != OK) { goto error; }

  goto everything_ok;

  error:
    /* error handling */

  everything_ok:

If neither of the above forms are OK because you are super-strict with MISRA-C, then the 3rd option might be something like this, which I believe is 100% MISRA-C compliant:

typedef type do_stuff_t (void);

do_stuff_t* const do_stuff[N] = { do_smth, do_smth2, do_smth3 };
type retval = OK;

for(uint32_t i=0u; (i<N) && (retval==OK); i++)
{
  retval = do_stuff[i]();
}

---

My point is that do { } while(0u); is not an iteration statement.

The C language disagrees with you.

¹⁾ From C17:

6.8.5 Iteration statements

Syntax

iteration-statement:
while ( expression ) statement
do statement while ( expression ) ;

Answer 2

I'd replace your code with this:

  retval = do_smth();
  if (retval == OK) {
    retval = do_smth2();
  } 
  if (retval == OK) {
    retval = do_smth3();
  }

• no phony while
• no gotos disguised as break
• hence not even a single goto/break
• hence no more MISRA issues
• bonus: half the number of lines than in the original code

BTW: the last break (break; // <- 3rd) was useless anyway

Answer 3

There should be no more than one break or goto statement used to terminate any iteration statement.

Your example has 3 breaks in one do-while (iteration statement), so it is not correct I think. A break is a control-flow/loop-control statement and is only valid in the context of a loop. I don't think your argument is valid here, though I see where you're going.

TL;DR: do-while is still an iteration statement, even though it only runs once.

do {
    retval = do_smth();
    if (retval != OK) {
        break; // <- 1st
    }

    retval = do_smth2();
    if (retval != OK) {
        break;  // <- 2nd
    }
    retval = do_smth3();
    if (retval != OK) {
        break; // <- 3rd
    }
} while (0u);

Answer 4

I don't know if this code is good enough. It works though.

switch(0) {
default:
    retval = do_smth();
    if (retval != OK) {
        break;
    }

    retval = do_smth2();
    if (retval != OK) {
        break;
    }

    retval = do_smth3();
    break;
}

Answer 5

I'm no expert on MISRA, so this may be forbidden for other reasons, but in this situation I might be tempted to use something like this:

if((retval = do_smth()) != OK)
    ;
else if((retval = do_smth2()) != OK)
    ;
else if((retval = do_smth3()) != OK)
    ;

No goto, no break, no iteration, no duplicated tests.

The empty statements look weird, it's true, although in practice, there will often be some error-logging code to put there. If naked semicolons aren't to your taste, you could use { }, or { /* do nothing */ }.

Assignments inside conditionals (as in the classic while((c = getchar()) != EOF) loop) can be confusing to the novice (and I suspect MISRA probably bans them for more or less that reason), but in many cases, if you can tolerate them, they can really help to make various other uglinesses melt away.