On Fre, 2010-02-19 at 13:10 +0300, Dan Carpenter wrote: > On Fri, Feb 19, 2010 at 09:29:05AM +0100, Clemens Ladisch wrote: > > > This doesn't change anything, but I think it makes the code clearer. > > > It silences a smatch warning: > > > sound/pci/oxygen/oxygen_mixer.c +91 dac_mute_put(7) warn: add some parenthesis here? > > > > That message doesn't say why some parentheses should be added. > > And it's a question; how do I give it the answer "no"? :-) > > > > > - changed = !value->value.integer.value[0] != chip->dac_mute; > > > + changed = (!value->value.integer.value[0]) != chip->dac_mute; > > > > This doesn't look any clearer to me; I don't think that the unary > > negation operator could be thought to have lower precedence than "!=". > > Well, it's hard to argue that it's more ambiguous. :P But it doesn't make the code clearer - unless you are a C novice. Unary operators generally bind stronger than others - be it "+", "-", "!", "~", "*". I would expect kernel programmers to know that (and I don't assume in-depth knowledge of operator precedence rules). > > Why does smatch warn about this combination? Do such errors actually > > happen: > > Yep. I have made some myself when writing smatch. > > For example here are some related bugs in the current kernel. > > drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c > 721 if (!ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY && Well, I see potential bugs here and the if() should have been a) if (!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY) && b) if (!ext->ext_flags && IW_ENCODE_EXT_GROUP_KEY && So you one has to look at the driver for the correct fix (and perhaps both of above are wrong). And I don't see what parenthesis around a logical negations can help with the above error example. Bernd -- Bernd Petrovitsch Email : bernd@xxxxxxxxxxxxxxxxxxx LUGA : http://www.luga.at -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html