Re: [alsa-devel] [patch] oxygen: clean up. make precedence explicit
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
On Fri, Feb 19, 2010 at 06:24:10PM +0100, Clemens Ladisch wrote:
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.)
Yup. The check already takes the type into account. Making chip->dac_mute type bool would silence the message.
regards, dan carpenter
Regards, Clemens
participants (2)
-
Clemens Ladisch -
Dan Carpenter