if/else block default coding style question

Nicholas Mc Guire der.herr at hofr.at
Sat Oct 8 11:23:49 EDT 2016


On Sat, Oct 08, 2016 at 11:10:10AM -0400, Robert P. J. Day wrote:
> On Sat, 8 Oct 2016, Valdis.Kletnieks at 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.
> 
>   if that's the behaviour the developer actually wants, then yes, it's
> messy. but i would be very careful just simplifying it wholesale,
> since it also smacks of a typo where one copy-and-pasted to add the
> default case, then forgot to tweak it to be different.
> 
>   rather than "fixing" it, i would bring it to the attention of the
> maintainer, and ask him or her to resolve it.
>
sure - no point in fixing code one does not understand.

if at all I send "fixes" out as RFCs if it seems like an obvious
case - and in all other cases a notification/questions are sent
but no patch.

thx!
hofrat
 



More information about the Kernelnewbies mailing list