drivers/iio/addac/ad74115.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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
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.
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á
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
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á
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
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
>
>
© 2016 - 2026 Red Hat, Inc.