C26430 TEST_ON_ALL_PATHS

"Symbol is not tested for nullness on all paths."

C++ Core Guidelines: F.23: Use a not_null<T> to indicate that "null" is not a valid value

If code ever checks nullness of pointer variables it should do this consistently and validate pointers on all paths. Sometimes overaggressive checking for nullness is still better than possibility of a hard crash in one of the complicated branches. Ideally such code should be refactored to be less complex (by splitting into multiple functions) and to rely on markers like gsl::not_null (see Guidelines Support Library) to isolate parts of algorithm that can make safe assumption about valid pointer values. The rule TEST_ON_ALL_PATHS helps to find places where nullness checks are either inconsistent (hence assumptions may require review) or actual bugs where potential null value can bypass nullness check in some of the code paths.

Remarks

  • This rule expects that code dereferences a pointer variable so that nullness check (or enforcement of non-null value) would be justified. If there is no dereference, the rule is suspended.
    • Current implementation handles only plain pointers (or their aliases) and doesn’t detect smart pointers even though nullness checks are applicable to smart pointers as well.
    • A variable is marked as checked for nullness when it is used in the following contexts:
    • as a symbol expression in a branch condition, e.g. "if (p) { ... }";
    • non-bitwise logical operations;
    • comparison operations where one operand is a constant expression which evaluates to zero.
    • The rule doesn’t have full data flow tracking and can produce incorrect results in cases where indirect checks are used (e.g. when intermediate variable holds null value and later used in comparison).
    • Implicit nullness checks are assumed when pointer value is assigned from:
    • an allocation performed with throwing operator new;
    • a pointer obtained from type marked with gsl::not_null.

Example

inconsistent testing reveals logic error

void merge_states(const state *left, const state *right) // C26430
{
    if (*left && *right)
        converge(left, right);
    else
    {
        // ...
        if (!left && !right)                            // Logic error!
            discard(left, right);
    }
}

inconsistent testing reveals logic error - corrected

void merge_states(gsl::not_null<const state *> left, gsl::not_null<const state *> right)
{
    if (*left && *right)
        converge(left, right);
    else
    {
        // ...
        if (*left && *right)
            discard(left, right);
    }
}