Re: [PATCH] iio: imu: adis: fix uninitialized symbol warning

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 4 Mar 2025 09:36:56 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:

> On Tue, Mar 04, 2025 at 02:05:18PM +0800, sunliming@linux.dev wrote:
> > From: sunliming <sunliming@kylinos.cn>
> > 
> > Fix below kernel warning:
> > smatch warnings:
> > drivers/iio/imu/adis.c:319 __adis_check_status() error: uninitialized symbol 'status_16'.
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Signed-off-by: sunliming <sunliming@kylinos.cn>  
> 
> Huh...  Someone is using lei to get their email.  This patch is fine and
> it's theoretically the correct thing to do.
> 
> How the zero-day bot warnings work is the they are first sent to my gmail
> account and I look them over and either forward them or ignore them.  Here
> is the code:
> 
> drivers/iio/imu/adis.c
>    305  int __adis_check_status(struct adis *adis)
>    306  {
>    307          unsigned int status;
>    308          int diag_stat_bits;
>    309          u16 status_16;
>    310          int ret;
>    311          int i;
>    312  
>    313          if (adis->data->diag_stat_size) {
>    314                  ret = adis->ops->read(adis, adis->data->diag_stat_reg, &status,
>    315                                        adis->data->diag_stat_size);
>    316          } else {
>    317                  ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg,
>    318                                           &status_16);
>    319                  status = status_16;
>    320          }
>    321          if (ret)
>    322                  return ret;
>    323  
> 
> So if __adis_read_reg_16() fails, then the next line is an uninitialized
> read.  But then the if (ret) check means that it's fine at run-time.
> It's a false positive.  The other thing to consider it the UBSan will
> also detect the uninitialized read at runtime and splat.  That's still a
> false positive but it's a headache.  But when I was looking at this, I
> decided that __adis_read_reg_16() was unlikely to fail in real life so I
> decided to ignore this warning.
> 
> Initializing the variable to zero doesn't change runtime for sane configs
> because everyone automatically zeroes stack variables these days.  It
> just silences the Smatch warning.  So I'm fine with this patch.
> 
> (This email is for information only in case you were wondering why the
> bug report was formatted strangely etc).
> 
> regards,
> dan carpenter
> 
Thanks!  That explanation has me agreeing that this patch seems to
make sense as fixing a warning that is reasonable if unlikely to
cause problems in practice. I've applied it to the togreg branch of iio.git 

Jonathan