do while(0) or if else which one is good for single exit point?

Question

I want to achieve single exit point for a function. One way is to use a variable, and check condition and if condition fails, set the variable to error and return that variable at the end of function. But nested if else hampers code readability.

Other way is to enclose the code in do {} while(0); and inside this loop check for condition and if condition fails, set the error in a variable and break from loop and then return the error, this improves the code readability.

So which one is better nested if else or do while(0)?

"I want to achieve single exit point for a function." -- Why? — Roger Lipscombe, May 19 '17 at 08:24
@RogerLipscombe and such guidelines often lead to "creative" replacements like `do .. break` for `goto` *shudder* — , May 19 '17 at 08:37
I would go with `if ... else` blocks. It will make your code hard to read if you have multiple levels of indentation, but if this happens you should move the nested code off to separate functions. The answers to [this question](http://stackoverflow.com/q/4838828/1679849) might be helpful. — r3mainer, May 19 '17 at 08:58
The correct approach in this case is to use multiple return statements and raise a MISRA deviation. See the linked duplicate regarding reasons why. — Lundin, May 19 '17 at 09:20

JFMR · Answer 1 · 2017-05-19T08:32:46.340

4

What about using the goto statement with a single label placed just before (or somewhere before) your return statement?

   //...
   // if (...)
      goto exit_func;
   //...
   // if (...)
      goto exit_func;
   //...
   // if (...)
      goto exit_func;
   //...
exit_func:
   // do some common stuff before leaving the function (e.g.: releasing resources)
   // ...
   return;

edited May 19 '17 at 08:32

answered May 19 '17 at 08:27

JFMR

23,265
4
52
76

2

+1 for this common idiom, IMHO good practice **if** there are resources to release on error conditions. Still I'm not sure whether [tag:misra] allows `goto` :o – May 19 '17 at 08:35
1

use of goto is not considered as good coding practice – Raghavendra Ganiga May 19 '17 at 08:50
Not if you are writing spaghetti code. Even K&R said that there are some scenarios in which `goto` is quite elegant, e.g.: *breaking from nested loops*. Having some common code to be executed every time you leave a function is another scenario. – JFMR May 19 '17 at 08:52
1

@RaviGaniga-- `goto` has its uses, and this is one of them. – ad absurdum May 19 '17 at 09:00
Depending on MISRA version, goto may or may not be banned. So this probably doesn't solve anything. – Lundin May 19 '17 at 09:14
Do not forget to put braces around your blocks, otherwise you'll end up repeating Apple's `goto fail` fubar – Andrew Jun 07 '17 at 15:04

score 1 · Answer 2 · answered May 19 '17 at 09:15

While you might not agree with the MISRA rules, you are not supposed to find creative attempt such as wrapping your code in do {} while(0); to achieve a single point of exit, or as a way to write less code.

MISRA forbids goto as well, so although it's a common idiom to use goto to jump to a common exit point in a function, it will not pass the MISRA rules.

Even if you find it verbose, you are supposed write the code as e.g.

int32 CFE_ES_GetAppInfo(CFE_ES_AppInfo_t *AppInfo, uint32 AppId)
{
   int32  ReturnCode = CFE_SUCCESS;

   if ( AppInfo != 0 )
   {
      if ( AppId < CFE_ES_MAX_APPLICATIONS )
      {
         if ( CFE_ES_Global.AppTable[AppId].RecordUsed == TRUE )
         {
            CFE_ES_GetAppInfoInternal(AppId, AppInfo);
            ReturnCode = CFE_SUCCESS;
         }
         else
         {
            CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: App ID Not Active: %d\n",(int)AppId);
            ReturnCode = CFE_ES_ERR_APPID;
         }
      }
      else
      {
         CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: App ID Exceeds CFE_ES_APPLICATION_MAX: %d\n",(int)AppId);
         ReturnCode = CFE_ES_ERR_APPID;
      }
   }
   else
   {
      CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: Invalid Parameter ( Null Pointer )\n");
      ReturnCode = CFE_ES_ERR_BUFFER;
   }
   return(ReturnCode);

} /* End of CFE_ES_GetAppInfo() */

And you are not supposed to find workarounds to avoid nested if statements to handle a single exit point.

If your code gets to many nested statements, you should rather break your function up into smaller pieces that each can handle its own part to reduce the nesting.

While the above code is verbose, it's still small enough to not need breaking up, but as a demonstration, it could be

static int32 CFE_ES_GetAppInfoImpl(CFE_ES_AppInfo_t *AppInfo, uint32 AppId)
{
   int32  ReturnCode;
   if ( CFE_ES_Global.AppTable[AppId].RecordUsed == TRUE )
   {
        CFE_ES_GetAppInfoInternal(AppId, AppInfo);
        ReturnCode = CFE_SUCCESS;
   }
   else
   {
        CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: App ID Not Active: %d\n",(int)AppId);
        ReturnCode = CFE_ES_ERR_APPID;
   }

   return ReturnCode;
}

int32 CFE_ES_GetAppInfo(CFE_ES_AppInfo_t *AppInfo, uint32 AppId)
{
   int32  ReturnCode = CFE_SUCCESS;

   if ( AppInfo != 0 )
   {
      if ( AppId < CFE_ES_MAX_APPLICATIONS )
      {
          ReturnCode = CFE_ES_GetAppInfoImpl(AppInfo, AppID);
      }
      else
      {
         CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: App ID Exceeds CFE_ES_APPLICATION_MAX: %d\n",(int)AppId);
         ReturnCode = CFE_ES_ERR_APPID;
      }
   }
   else
   {
      CFE_ES_WriteToSysLog("CFE_ES_GetAppInfo: Invalid Parameter ( Null Pointer )\n");
      ReturnCode = CFE_ES_ERR_BUFFER;
   }
   return(ReturnCode);

} /* End of CFE_ES_GetAppInfo() */

MISRA C:2012 does NOT forbid the use of `goto` - but it does provide guidance of how `goto` can be used safely. — Andrew, Jun 07 '17 at 15:05

do while(0) or if else which one is good for single exit point?

2 Answers2