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

[Purva Yeshi <purvayeshi550@gmail.com>]

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@gmail.com>
>>> ---
>>>   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;


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.

Best regards,
Purva