Refactoring of a possibly unsafe pattern for variable initialization via function calls

[nicola <nicola.vetrini@xxxxxxxxxxx>]

Hi all,

while investigating possible patches regarding Mandatory Rule 9.1, I
found the following pattern, that is likely to results in a lot possible
positives from many (all) static analysis tools for this rule.

This is the current status (taken from `xen/common/device_tree.c:135')


const struct dt_property *dt_find_property(const struct dt_device_node *np,
                                           const char *name, u32 *lenp)
{
    const struct dt_property *pp;

    if ( !np )
        return NULL;

    for ( pp = np->properties; pp; pp = pp->next )
    {
        if ( dt_prop_cmp(pp->name, name) == 0 )
        {
            if ( lenp )
                *lenp = pp->length;
            break;
        }
    }

    return pp;
}




It's very hard to detect that the pointee is always written whenever a 
non-NULL pointer for `lenp' is supplied, and it can safely be read in 
the callee, so a sound analysis will err on the cautious side.
My proposal, in a future patch, is to refactor these kinds of functions 
as follows:

const struct dt_property *dt_find_property(const struct dt_device_node *np,
                                           const char *name, u32 *lenp)
{
    u32 len = 0;
    const struct dt_property *pp;

    if ( !np )
        return NULL;

    for ( pp = np->properties; pp; pp = pp->next )
    {
        if ( dt_prop_cmp(pp->name, name) == 0 )
        {
            len = pp->length;
            break;
        }
    }

    if ( lenp )
        *lenp = len;
    return pp;
}


The advantage here is that we can easily argue that `*lenp' is always
initialized by the function (if not NULL) and inform the tool about
this, which is a safer API and also resolves almost all subsequent
"don't know"s about further uses of the variables involved (e.g. `lenp').

Regards,
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)