linux-kernel - Re: [PATCH] Revert "Staging: iio: adt7316: Add an extra check for 'ret' equals to 0"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Dec 2018 14:59:53 -0700
From:   Jeremy Fertic <jeremyfertic@...il.com>
To:     Shreeya Patel <shreeya.patel23498@...il.com>
Cc:     jic23@...nel.org, linux-iio@...r.kernel.org, lars@...afoo.de,
        Michael.Hennerich@...log.com, knaack.h@....de, pmeerw@...erw.net,
        gregkh@...uxfoundation.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "Staging: iio: adt7316: Add an extra check for
 'ret' equals to 0"

On Thu, Dec 06, 2018 at 01:25:55AM +0530, Shreeya Patel wrote:
> On Tue, 2018-12-04 at 18:49 -0700, Jeremy Fertic wrote:
> > This reverts commit 00426e99789357dbff7e719a092ce36a3ce49d94.
> > 
> > i2c_smbus_read_byte() returns 0 when a byte with the value 0 is read
> > from
> > the device. This is a valid read so revert the check for 0.
> > 
> > Signed-off-by: Jeremy Fertic <jeremyfertic@...il.com>
> > ---
> 
> Hi Jeremy,
> 
> As per my understanding, 0 value indicates no error but no data read.
> Then how can this be a valid case?
> 
> Can you please make me understand that how can we consider this as a
> valid case even when no data has been read?
> 
> 
> Thanks

I'm not sure I understand why the value 0 would indicate no data read.
Doesn't that just mean a byte was read with the value 0. For instance,
if the input to the adc is 0V. Can you point me to where you're seeing
that this would indicate no data read?

> 
> >  drivers/staging/iio/addac/adt7316-i2c.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
> > b/drivers/staging/iio/addac/adt7316-i2c.c
> > index ac91163656b5..2d51bd425662 100644
> > --- a/drivers/staging/iio/addac/adt7316-i2c.c
> > +++ b/drivers/staging/iio/addac/adt7316-i2c.c
> > @@ -30,10 +30,6 @@ static int adt7316_i2c_read(void *client, u8 reg,
> > u8 *data)
> >  	}
> >  
> >  	ret = i2c_smbus_read_byte(client);
> > -
> > -	if (!ret)
> > -		return -EIO;
> > -
> >  	if (ret < 0) {
> >  		dev_err(&cl->dev, "I2C read error\n");
> >  		return ret;