Messages in this thread | | | Subject | Re: [PATCH] hwmon: nct7904: fix error check on register read | From | Guenter Roeck <> | Date | Thu, 6 Jun 2019 06:02:07 -0700 |
| |
Hi Colin,
On 6/6/19 5:27 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Currently the return from the call to nct7904_read is being masked > and so and negative error returns are being stripped off and the > error check is always false. Fix this by checking on err first and > then masking the return value in ret. > > Addresses-Coverity: ("Logically dead code") > Fixes: af55ab0b0792 ("hwmon: (nct7904) Add extra sysfs support for fan, voltage and temperature.") > Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks a lot for the patch. The offending patch had several additional problems (shame on me for sloppy review), so I pulled it from the branch and asked the author to fix all problems and resubmit.
Guenter
> --- > drivers/hwmon/nct7904.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c > index dd450dd29ac7..5fa69898674c 100644 > --- a/drivers/hwmon/nct7904.c > +++ b/drivers/hwmon/nct7904.c > @@ -928,10 +928,10 @@ static int nct7904_probe(struct i2c_client *client, > > /* Check DTS enable status */ > if (data->enable_dts) { > - ret = nct7904_read_reg(data, BANK_0, DTS_T_CTRL0_REG) & 0xF; > + ret = nct7904_read_reg(data, BANK_0, DTS_T_CTRL0_REG); > if (ret < 0) > return ret; > - data->has_dts = ret; > + data->has_dts = ret & 0xF; > if (data->enable_dts & ENABLE_TSI) { > ret = nct7904_read_reg(data, BANK_0, DTS_T_CTRL1_REG); > if (ret < 0) >
| |