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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1365593271.27174.35.camel@joe-AO722>
Date:	Wed, 10 Apr 2013 04:27:51 -0700
From:	Joe Perches <joe@...ches.com>
To:	Andy Whitcroft <apw@...dowen.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: Re: [PATCH] checkpatch: Warn on comparisons to true and false

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/