#1241 Findbugs not parsing exclusive or

[Stephen Chester]

In findbugs 2.0.3 the following code generates a 'Possible null pointer dereference on path that might be infeasible' bug at the call to String.compareTo(). I believe this to be invalid, as the combination of 'exclusive or (^)' and 'and (&&)' operators should ensure that neither String can possibly be null at that point.

private static int nullSafeStringComparator(final String one, final String two)
{
    if (one == null ^ two == null)
    {
        return (one == null) ? -1 : 1;
    }

    if (one == null && two == null)
    {
        return 0;
    }

    return one.compareTo(two); //Bug found on this line
}

[William Pugh]

The warning is labeled as:
Possible null pointer dereference of one on branch that might be infeasible

And is marked as a style issue, rather than a correctness issue, due to FindBugs lack of confidence in the report.

In fact, it would be better to report it as a redundant check for null, since checking both parts of (one == null && two == null) is redundant. Since you've already checked for the xor case, you know that if one of them is null, the other must be as well. You could change that check to (one == null), (two == null), or (one == two), and your code would still be correct and the null pointer warning would go away.

[Stephen Chester]

Thanks for the quick response.

There definitely is a redundant check for null in there. I would still, however, suggest that both the location where findbugs reported the issue and its name are confusing.

[William Pugh]

The message generated by FindBugs is exactly correct: possible null dereference on path that may be infeasible. Looking at the code, the path in infeasible (the false branch from the test for two == null, after already finding that one == null, is guaranteed to be infeasible, and thus the test is redundant).

FindBugs doesn't use a theorem prover. It will always be possible to encode logic puzzles it can't solve. This isn't a case were are going to try to address, particularly since the message already generated is correct and descriptive.

Bill

On Jan 9, 2014, at 10:02 PM, Stephen Chester chesterm8@users.sf.net wrote:

Thanks for the quick response.

There definitely is a redundant check for null in there. I would still, however, suggest that both the location where findbugs reported the issue and its name are confusing.

[bugs:#1241] Findbugs not parsing exclusive or

Status: pending-rejected
Labels: false positive
Created: Thu Jan 09, 2014 03:58 AM UTC by Stephen Chester
Last Updated: Thu Jan 09, 2014 01:49 PM UTC
Owner: William Pugh

In findbugs 2.0.3 the following code generates a 'Possible null pointer dereference on path that might be infeasible' bug at the call to String.compareTo(). I believe this to be invalid, as the combination of 'exclusive or (^)' and 'and (&&)' operators should ensure that neither String can possibly be null at that point.

private static int nullSafeStringComparator(final String one, final String two)
{
if (one == null ^ two == null)
{
return (one == null) ? -1 : 1;
}

if (one == null && two == null)
{
    return 0;
}

return one.compareTo(two); //Bug found on this line

}
Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/findbugs/bugs/1241/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/