C# Coverity giving "Dereference before null check (REVERSE_INULL)" on a RemoveAll call to remove null entries from a list

Question

 public void UpdateCredentialSelect(ClientCredentials credential, bool selected)
{
    onsSelectedCredentials.RemoveAll(x => x.Equals(null));
    if (selected && !onsSelectedCredentials.Exists(x => x.name.Equals(credential.name)))
    {
        onsSelectedCredentials.Add(credential);
    }
    else
    {
        onsSelectedCredentials.Remove(credential);
    }

    onsSecurityScreen.UpdateDynamicItems();
    onsSecurityScreen.UpdateSelectAllCheckmark();
}

Running through Coverity reports and it is having issues with the "onsSelectedCredentials.RemoveAll(x => x.Equals(null));" line here, stating "check_after_deref: Null-checking x suggests that it may be null, but it has already been dereferenced on all paths leading to the check." The purpose of that line of code is to read through the current values in the list and strip out any that have become null, there's no null check happening as far as I can tell. Is this a false positive from Coverity or should I do something to fix this?

Not sure if this is directly related to the error/warning you get but in general I think you should rather use `x == null` instead (see e.g. [Equals(item, null) or item == null](https://stackoverflow.com/questions/3507383/equalsitem-null-or-item-null)) — derHugo, Mar 29 '21 at 19:56

score 0 · Answer 1 · answered Mar 30 '21 at 02:40

The expression x.Equals(null) will throw NullReferenceException if x is null. It can never evaluate to true (unless Equals has been overridden to do something screwy).

Coverity is correctly telling you that, albeit in a somewhat indirect way. Specifically, it understands that Equals is meant to test equality, and that you're comparing x to null as if they might be the same (the "check"), but you can't get into Equals (the "path") at all because of the NullReferenceException. It calls x.Equals() a "dereference", unfortunately using C/C++ terminology (for historical reasons).

To fix the bug in the code and also make Coverity happy, as suggested by derHugo in a comment, change the RemoveAll line to something like this:

onsSelectedCredentials.RemoveAll(x => (x == null));

C# Coverity giving "Dereference before null check (REVERSE_INULL)" on a RemoveAll call to remove null entries from a list

1 Answers1