On Sat, Oct 08, 2016 at 10:58:41AM -0400, Valdis.Kletnieks@vt.edu wrote:
On Sat, 08 Oct 2016 10:40:37 -0000, Nicholas Mc Guire said:
} else if (rtlpcipriv->bt_coexist.bt_service == BT_PAN) { rtl_write_byte(rtlpriv, REG_GPIO_MUXCFG, tmp1byte); } else { rtl_write_byte(rtlpriv, REG_GPIO_MUXCFG, tmp1byte); }
That *does* smell like a bug. If nothing else, the last 'else if' can be removed. Most likely case: somebody cut-n-pasted that last section in and failed to change it to a proper 'default' value and the code falls through to that one rarely enough that nobody has noticed.
I personally find this irritating as (without a comment) it is hard to say if this is a trivial type -> missed case, or if this is intended as a default behavior.
Unfortunately, it will probably be a really rough job figuring out what exactly was intended in each case. Figuring it out in filesystem code or other similar areas of the kernel wouldn't be too bad - but if it's a hardware driver, you're going to have to ask an expert (or somebody who has the hardware) for help.
Actually fs had one legitimate case where the positional side-effect was in use <snip: fs/kernfs/file.c:kernfs_fop_open()> * Both paths of the branch look the same. They're supposed to * look that way and give @of->mutex different static lockdep keys. */ if (has_mmap) mutex_init(&of->mutex); else mutex_init(&of->mutex); <snip> but almost all of the other cases look phony
How did you find there were 90 of them? Did you cook up a Coccinelle script for it?
simple cocci script - Im checking through the reported cases to see if it is actually reliable. In some cases it was obvious bugs (cut&paste type) but in others it does seem to be a "coding pattern".
If nothing else, cooking up a test to toss into scripts/coccinelle/misc would probably be a Good Thing...
Thats actually the cause of this mail - it just becaem obvious that some of these if==else are intentional Now reporting these cases if they should be fixed makes sense while if this is accepted practice then it would be reporting false-positives, which would be bad. thx! hofrat