cmisracert

if/else strive for logical completeness in single return function


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):

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?


Solution

  • 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:

    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.