[alsa-devel] [patch] oxygen: clean up. make precedence explicit

Clemens Ladisch clemens at ladisch.de
Fri Feb 19 18:24:10 CET 2010


Bernd Petrovitsch wrote:
> On Fre, 2010-02-19 at 14:29 +0300, Dan Carpenter wrote:
> > On Fri, Feb 19, 2010 at 11:33:30AM +0100, Bernd Petrovitsch wrote:
> > Basically often when people write:
> > 	if (!foo == bar) { ...
> > 
> > What they mean is:
> > 	if (!(foo == bar)) { ...

But there are also cases where they mean what they've written.

> Ugh. The IMHO better way is 
> 	if (foo != bar) { ...

In my case, the driver compares an "enabled" variable against a
"disabled" one; negating the comparison operator would obfuscate the
logic.

> > But if they really do mean the original code they could just write 
> > this so it's clear to everyone: 
> > 	if ((!foo) == bar) { ...

This is unnatural (especially in a simple example like this) because
the parens haven't been needed at all before smatch.


!foo==bar is always identical to !(foo==bar) for boolean values; to
avoid false positives, you could output the warning only when the code
is trying to manipulate non-boolean values.  IMO the message would be
justified if it said "using suspicious boolean operations on non-boolean
types".  (In fact, my driver uses types long and u8 in this expression,
so I will clean it up.)


Regards,
Clemens


More information about the Alsa-devel mailing list