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

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

[sunliming]

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>
---
 drivers/iio/imu/adis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 1c646c36aeb1..0ea072a4c966 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -306,7 +306,7 @@ int __adis_check_status(struct adis *adis)
 {
 	unsigned int status;
 	int diag_stat_bits;
-	u16 status_16;
+	u16 status_16 = 0;
 	int ret;
 	int i;
 
-- 
2.25.1

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

[Dan Carpenter]

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

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

[Jonathan Cameron]

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