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

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

[Purva Yeshi]

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)
-- 
2.34.1

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

[David Lechner]

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.

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

[Purva Yeshi]

On 10/04/25 20:21, 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.

Hi,

Thank you for the review and explanation.

Understood — since there's a risk of misuse later (like divide-by-zero), 
it's best to leave it as is.

Best regards,
Purva

> 
> 

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

[Nuno Sá]

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á 

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

[Purva Yeshi]

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

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

[Nuno Sá]

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@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;
> 

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á

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@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;
>>
> 
> 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