Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Tags:

c

misra

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?

like image 954
radoslav006 Avatar asked Dec 09 '22 23:12

radoslav006


2 Answers

First of all, your code does indeed not follow rule 15.4 since you have 3 break inside an iteration statement1). 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.

1) From C17:

6.8.5 Iteration statements

Syntax

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

like image 164
Lundin Avatar answered Dec 29 '22 11:12

Lundin


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

like image 26
Jabberwocky Avatar answered Dec 29 '22 11:12

Jabberwocky