Re: [PATCH] checkpatch: Warn on comparisons to true and false

[Joe Perches]

On Wed, 2013-04-10 at 10:33 +0100, Andy Whitcroft wrote:
> On Tue, Apr 09, 2013 at 08:17:14PM -0700, Joe Perches wrote:
> > Comparisons of A to true and false are better written
> > as A and !A.
[]
> In a complex case such as  a + b == false  will this do the right thing?

Very sensible question.  No it won't.

checkpatch doesn't understand expressions
very well nor does it understand precedence
operations between expressions and tests.

I did run it against the kernel tree and there
weren't any I noticed like that though.

> Not that I am sure that adding bools makes sense but assuming there is
> some valid complex lval.

$Lval in this case is a single variable and
is neither a function nor an expression.

It doesn't match on:

	if (func(x, y) == true)
or
	if ((x | y) == true)

because of the ) before the ==

It will falsely match on expressions like:

	if (x + y == true)

but as far as I can tell there aren't any
uses like that in the kernel tree.

$ git grep -E  "(==|\!=)\s*(true|false)\b" | \
  cut -f2- -d":" |grep -P "\b(\+|\-)\b"

nor are there any and/or bit operator.

When I tried adding a test for:

	"$Constant == $Lval"
instead of
	"$Lval == $Constant"

like
	0 == foo
instead of
	foo == 0

there were _way_ too many false positives of
the $Expression sort that I didn't add that test.

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/