Re: [patch] oxygen: clean up. make precedence explicit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux