if/else block default coding style question

[Valdis.Kletnieks at vt.edu]

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.

How did you find there were 90 of them?  Did you cook up a Coccinelle script
for it?

If nothing else, cooking up a test to toss into scripts/coccinelle/misc
would probably be a Good Thing...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 484 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20161008/4a6c5495/attachment.bin