if/else strive for logical completeness in single return function

Question

I'm attempting to exist at the crossroads of MISRA C and CERT C and setting the lofty goal of no exceptions. The two rules most against my normal patterns are (paraphrased):

• MISRA : A function should only have one return
• CERT : Strive for logical completeness

The CERT rule keeps catching me when I have nothing to say in an else. For example:

static int32_t SomeFunc()
{
    int32_t retval = PROJECT_ERROR_GENERIC;

    retval = ChildFuncOne();
    if (retval == PROJECT_SUCCESS)
    {
        retval = ChildFuncTwo();
    }

    //Common cleanup

    return retval;
}

Assuming there is no specific cleanup for the failure of ChildFuncOne, I have nothing to say in else. Am I missing another way to lay out this function?

Answer 1

The only problem with your code I can find is the obsolescent style function definition. static int32_t SomeFunc() should be static int32_t SomeFunc(void). The former has been obsolescent form in C since some 25-30 years back and also violates MISRA 8.2 for that reason. Other than that, the code is fine and MISRA compliant.

A style comment beyond MISRA is that I would use an enum over int32_t.

---

Regarding CERT I assume you refer to MSC01-C. This is a fuzzy and unclear rule, with just a bunch of non-related code examples. I would just ignore it.

The corresponding MISRA C:2012 15.7 and 16.1 are much clearer and can be summarized as:

• All else if should end with an else.
• All switch should contain default.

The rationale is defensive programming and self-documenting code: "yes, I have definitely considered this outcome". Likely this is what CERT C tried (and failed) to say as well. They refer to various "else if with else" rules in their crossref.

This isn't applicable to your code. Nobody requires that every if ends with an else, that would be a ridiculous requirement.

---

Am I missing another way to lay out this function?

It's sound practice to set the result variable to a known default state at the beginning of a function and later overwrite it if needed. So your function is not only compliant, but also fairly canonical style as safety-related programming goes.

So other than the () to (void), there's no need to change a thing.

Answer 2

Option 1 is an else with an empty body:

static int32_t SomeFunc(void)
{
    int32_t retval = PROJECT_ERROR_GENERIC;

    retval = ChildFuncOne();
    if (retval == PROJECT_SUCCESS)
    {
        retval = ChildFuncTwo();
    }
    else
    {
        // yes, I did consider every possible retval
    }

    //Common cleanup

    return retval;
}

Option 2 is to add a second variable, and then set that variable in the if and the else. Note that I reversed the sense of the if, since that order makes more sense to me. YMMV.

static int32_t SomeFunc2(void)
{
    int32_t retval = PROJECT_ERROR_GENERIC;
    int32_t finalretval = PROJECT_ERROR_GENERIC;

    retval = ChildFuncOne();
    if (retval != PROJECT_SUCCESS)
    {
        finalretval = retval;
    }
    else
    {
        finalretval = ChildFuncTwo();
    }

    //Common cleanup

    return finalretval;
}

The problem with option 2 is that it's easy to mix up the two variables that have similar names and uses. Which is where these coding standards make your code more likely to have bugs, rather than less.

Answer 3

My attempt:

static int32_t SomeFunc()
{
    int32_t retval = ChildFuncOne();
    retval = (retval == PROJECT_SUCCESS)? ChildFuncTwo()   : retval;
    retval = (retval == PROJECT_SUCCESS)? ChildFuncThree() : retval;

    return retval;
}

Basically, the retval is set by the first function, and only if that result is PROJECT_SUCCESS, will the second function get called and set the retval.

If the retval is anything other than success, it remains unchanged, the second function is never called, and retval is returned.

I even show how it can be chained for an arbitrary number of functions.

I'm a bit unclear what you mean bye "common cleanup", so if you need different cleanup operations depending on what functions succeeded and failed, that will take extra work.

Answer 4

You should stop trying to appease the MISRA and CERT gods and just write clear code. The code that you posted is fine. Keep it that way.