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.