Re: [PATCH 08/10] iio: imu: adis16480: assign bias value only if operation succeeded

[Jonathan Cameron]

On Fri, 1 Nov 2019 11:35:03 +0200
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:

> This was found only after the whole thing with the inline functions, but
> the compiler actually found something. The value of the `bias` (in
> adis16480_get_calibbias()) should only be set if the read operation was
> successful.
> 
> Fixes: 2f3abe6cbb6c9 ("iio:imu: Add support for the ADIS16480 and similar IMUs")
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>

Hmm. It's not really a fix as such unless there is an in kernel consumer
that is using this and not checking the return value.   I thought about dropping
the fixes tag, but it is avoiding confusing the compiler so I suppose we might
as well leave it marked as a fix.  I will add a note that this doesn't cause
any known real problems and so probably isn't stable material.

Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16480.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index b9e2695bcfad..c0e7e768be41 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -451,12 +451,14 @@ static int adis16480_get_calibbias(struct iio_dev *indio_dev,
>  	case IIO_MAGN:
>  	case IIO_PRESSURE:
>  		ret = adis_read_reg_16(&st->adis, reg, &val16);
> -		*bias = sign_extend32(val16, 15);
> +		if (ret == 0)
> +			*bias = sign_extend32(val16, 15);
>  		break;
>  	case IIO_ANGL_VEL:
>  	case IIO_ACCEL:
>  		ret = adis_read_reg_32(&st->adis, reg, &val32);
> -		*bias = sign_extend32(val32, 31);
> +		if (ret == 0)
> +			*bias = sign_extend32(val32, 31);
>  		break;
>  	default:
>  		ret = -EINVAL;