On Tue, Jun 02, 2015 at 11:23:41PM +0300, Vladimir Zapolskiy wrote: > On 02.06.2015 22:45, Mark Brown wrote: > > On Tue, Jun 02, 2015 at 02:09:16AM +0300, Vladimir Zapolskiy wrote:
> >> + unsigned int val = 0; > >> + > >> + if (value) > >> + val = 0x1 << WM5100_GP1_LVL_SHIFT; > > Write this as an if/else so the reader doesn't have to wonder why you've > > missed the handling of the false case. > the only objection I have is that the resulting code will be two lines > longer. If you think this code is not clear enough (is "val" vs. "value" > misleading?), I'll change the rest of my patches, which contain the same > logic structure, please let me know. Especially after the unrelated style change thing earlier on (which meant I was reading things more carefully than usual) it'd be good to make things as clear as possible - you're right that the val vs value thing isn't helping either. Like I say I am a bit surprised that the int/bool conversion doesn't do the right thing without any code changes other than the type of the parameter but ICBW, I didn't actually check.
signature.asc
Description: Digital signature
