Re: [PATCH 2/2] lm90.c: fix checkpatch error

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

 



On Wed, 4 Jan 2012 14:29:05 -0800, Guenter Roeck wrote:
> On Wed, 2012-01-04 at 17:27 -0500, Frans Meulenbroeks wrote:
> > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@xxxxxxxxx>
> > ---
> >  drivers/hwmon/lm90.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> > index 615bc4f..36dcbc9 100644
> > --- a/drivers/hwmon/lm90.c
> > +++ b/drivers/hwmon/lm90.c
> > @@ -388,9 +388,14 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
> >  	 * we have to read the low byte again, and now we believe we have a
> >  	 * correct reading.
> >  	 */
> > -	if ((err = lm90_read_reg(client, regh, &oldh))
> > -	 || (err = lm90_read_reg(client, regl, &l))
> > -	 || (err = lm90_read_reg(client, regh, &newh)))
> > +	err = lm90_read_reg(client, regh, &oldh);
> > +	if (err)
> > +		return err;
> > +	err = lm90_read_reg(client, regl, &l);
> > +	if (err)
> > +		return err;
> > +	err = lm90_read_reg(client, regh, &newh);
> > +	if (err)
> >  		return err;
> >  	if (oldh != newh) {
> >  		err = lm90_read_reg(client, regl, &l);
> 
> Ah, we left those in for readability.

Actually this is one of the things checkpatch complains about which I
consider a false positive. There is really nothing wrong with the code
above, it does the right thing with no risk of error. Its replacement,
OTOH, is harder to read, and less efficient.

The check should either be improved to only complain in cases which are
really wrong, or should be plain discarded if fixing it is not possible.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux