Re: [PATCH V4] hwmon: add fan/pwm driver for corsair h100i platinum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> [...]
> >> +			if (val < (2 ^ 16) - 2)
> >
> > Did you intend to write 2 to the power of 16? Because 2^16 is not that.
> > 2 to the power 16 may be written as '(1 << 16)' or 'BIT(16)'
> > (you may need to include linux/bits.h for the latter)
> >
> > But something like this is my suggestion:
> >
> > if (val < 0 || 0xFFFF < val)
> > 	return -EINVAL;
> >
>
> I would suggest to use clamp_val; we don't expect users to know device
> specific limits. Also, FTR, I won't accept any Yoda programming.
>
> Guenter
> [...]


If you don't mind me asking, why is that? I, too, don't like Yoda conditions,
but I've always thought that in this specific case - when checking if a value
lies within/outside a certain range - it makes the code more easily readable
if it is written like this: (lo < val && val < hi).

Maybe I'm just too accustomed to it?

(Don't get me wrong, I'm not trying to argue with your decision about not
accepting such code, neither am I trying to convince you otherwise.)


Barnabás Pőcze




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux