Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate

[Purva Yeshi]

On 11/04/25 17:47, Nuno Sá wrote:
> On Fri, 2025-04-11 at 14:39 +0530, Purva Yeshi wrote:
> > On 11/04/25 11:19, Nuno Sá wrote:
> > > On Thu, 2025-04-10 at 09:51 -0500, David Lechner wrote:
> > > > On 4/9/25 3:29 PM, Purva Yeshi wrote:
> > > > > Fix Smatch-detected error:
> > > > > drivers/iio/addac/ad74115.c:823 _ad74115_get_adc_code() error:
> > > > > uninitialized symbol 'rate'.
> > > > >
> > > > > The variable rate was declared but not given any value before being used
> > > > > in a division. If the code reached that point without setting rate, it
> > > > > would cause unpredictable behavior.
> > > > >
> > > > > Declare and initialize 'rate' to zero inside the 'else' block where it
> > > > > is
> > > > > used. This ensures 'rate' is always initialized before being passed to
> > > > > DIV_ROUND_CLOSEST.
> > > > >
> > > > > Signed-off-by: Purva Yeshi <purvayeshi550@xxxxxxxxx>
> > > > > ---
> > > > >    drivers/iio/addac/ad74115.c | 2 +-
> > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/iio/addac/ad74115.c b/drivers/iio/addac/ad74115.c
> > > > > index a7e480f2472d..26770c68e5fa 100644
> > > > > --- a/drivers/iio/addac/ad74115.c
> > > > > +++ b/drivers/iio/addac/ad74115.c
> > > > > @@ -814,7 +814,7 @@ static int _ad74115_get_adc_code(struct
> > > > > ad74115_state *st,
> > > > >    			return -ETIMEDOUT;
> > > > >    	} else {
> > > > >    		unsigned int regval, wait_time;
> > > > > -		int rate;
> > > > > +		int rate = 0;
> > > > >    
> > > > >    		ret = ad74115_get_adc_rate(st, channel, &rate);
> > > > >    		if (ret < 0)
> > > >
> > > > I don't see how rate could be used uninitialized since we are
> > > > returning the error if ad74115_get_adc_rate() fails.
> > > >
> > > > Also, initializing to 0 would then cause a divide by 0 error
> > > > if that value was actually used later in the code.
> > >
> > > Agreed... A better check could actually be (in ad74115_get_adc_rate()):
> > >
> > >
> > > if (i >= ARRAY_SIZE(ad74115_get_adc_rate))
> > >       return -EIO;
> > >
> > > Kind of a paranoid check but just making sure a faulty chip does not lead to
> > > an out
> > > of bounds access.
> > >
> > > - Nuno Sá
> >
> > Hi Nuno,
> >
> > Thank you for your suggestion regarding the paranoid check.
> >
> > However, ad74115_get_adc_rate is a function, not an array, pointer, or
> > vector. Therefore, using ARRAY_SIZE on it results in a compilation error.
> >
> > I believe the intended check was:
> >
> > if (i >= ARRAY_SIZE(ad74115_adc_conv_rate_tbl))
> >       return -EIO;
>
> Oh yes, bad copy-paste...
>
> > This ensures that the index i does not exceed the bounds of the
> > ad74115_adc_conv_rate_tbl array, preventing potential out-of-bounds access.
> >
> > This check prevents potential out-of-bounds access, it does not address
> > the Smatch warning about the uninitialized variable 'rate'. Smatch may
> > still flag 'rate' as potentially uninitialized if it cannot determine
> > that ad74115_get_adc_rate() always initializes it before use.
>
> Well, as said, this is a false positive...
>
> - Nuno Sá

Hi Nuno,

Thank you for the review. I'll drop the patch.

Best regards,
Purva