if/else strive for logical completeness in single return function-CodePudding

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?

CodePudding user response：

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.

CodePudding user response：

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.

CodePudding user response：

I'm not fond of trying to appease both MISRA and CERT but this should do it:

static int32_t SomeFunc()
{
    int32_t retval = (ChildFuncOne() == PROJECT_SUCCESS) ? ChildFuncTwo()
                                                         : PROJECT_ERROR_GENERIC;
    
    // cleanup
    
    return retval;
}

This would however return PROJECT_ERROR_GENERIC if ChildFuncOne() does not return PROJECT_SUCCESS, so that's perhaps not what you want.