* [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
@ 2023-03-14 19:37 Kasumov Ruslan
2023-03-14 20:42 ` Andi Shyti
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Kasumov Ruslan @ 2023-03-14 19:37 UTC (permalink / raw)
To: Andy Gross
Cc: Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij, linux-arm-msm, linux-iio,
linux-kernel, lvc-project, Kasumov Ruslan
The left side of the loop condition never becomes false.
hwchan cannot be NULL, because it points to elements of the
hw_channels array that takes one of 4 predefined values:
pm8018_xoadc_channels, pm8038_xoadc_channels,
pm8058_xoadc_channels, pm8921_xoadc_channels.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
---
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index eb424496ee1d..64a3aeb6261c 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -758,7 +758,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
/* Find the right channel setting */
chid = 0;
hwchan = &hw_channels[0];
- while (hwchan && hwchan->datasheet_name) {
+ while (hwchan->datasheet_name) {
if (hwchan->pre_scale_mux == pre_scale_mux &&
hwchan->amux_channel == amux_channel)
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 19:37 [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel() Kasumov Ruslan
@ 2023-03-14 20:42 ` Andi Shyti
2023-03-14 21:12 ` Linus Walleij
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-03-14 20:42 UTC (permalink / raw)
To: Kasumov Ruslan
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij, linux-arm-msm, linux-iio,
linux-kernel, lvc-project, Kasumov Ruslan
On Tue, Mar 14, 2023 at 10:37:09PM +0300, Kasumov Ruslan wrote:
> The left side of the loop condition never becomes false.
> hwchan cannot be NULL, because it points to elements of the
> hw_channels array that takes one of 4 predefined values:
> pm8018_xoadc_channels, pm8038_xoadc_channels,
> pm8058_xoadc_channels, pm8921_xoadc_channels.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
It wasn't broken before, it wasn't causing any harm. It's not a
fix, it's a cleanup. Please, remove remove the "Fixes:" tag.
> Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
> ---
> drivers/iio/adc/qcom-pm8xxx-xoadc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> index eb424496ee1d..64a3aeb6261c 100644
> --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> @@ -758,7 +758,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
> /* Find the right channel setting */
> chid = 0;
> hwchan = &hw_channels[0];
> - while (hwchan && hwchan->datasheet_name) {
> + while (hwchan->datasheet_name) {
With the fixes tag removed:
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Thanks,
Andi
> if (hwchan->pre_scale_mux == pre_scale_mux &&
> hwchan->amux_channel == amux_channel)
> break;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 19:37 [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel() Kasumov Ruslan
2023-03-14 20:42 ` Andi Shyti
@ 2023-03-14 21:12 ` Linus Walleij
2023-03-14 21:28 ` Andi Shyti
2023-03-14 22:04 ` Linus Walleij
2023-03-14 22:16 ` Linus Walleij
2023-03-15 13:51 ` [PATCH v2] " Kasumov Ruslan
3 siblings, 2 replies; 11+ messages in thread
From: Linus Walleij @ 2023-03-14 21:12 UTC (permalink / raw)
To: Kasumov Ruslan
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, linux-arm-msm, linux-iio, linux-kernel,
lvc-project, Kasumov Ruslan
On Tue, Mar 14, 2023 at 8:37 PM Kasumov Ruslan <xhxgldhlpfy@gmail.com> wrote:
> The left side of the loop condition never becomes false.
> hwchan cannot be NULL, because it points to elements of the
> hw_channels array that takes one of 4 predefined values:
> pm8018_xoadc_channels, pm8038_xoadc_channels,
> pm8058_xoadc_channels, pm8921_xoadc_channels.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
I am not impressed with that tool. See below:
> Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
> Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
(...)
> hwchan = &hw_channels[0];
> - while (hwchan && hwchan->datasheet_name) {
> + while (hwchan->datasheet_name) {
> if (hwchan->pre_scale_mux == pre_scale_mux &&
> hwchan->amux_channel == amux_channel)
> break;
NAK have you tested this on a real system?
Here is the complete loop:
hwchan = &hw_channels[0];
while (hwchan && hwchan->datasheet_name) {
if (hwchan->pre_scale_mux == pre_scale_mux &&
hwchan->amux_channel == amux_channel)
break;
hwchan++;
chid++;
}
Notice how hwchan is used as iterator in hwchan++.
What you are doing will cause a zero-pointer dereference.
Whoever is developing "SVACE" needs to fix the tool to understand
this kind of usage.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 21:12 ` Linus Walleij
@ 2023-03-14 21:28 ` Andi Shyti
2023-03-14 22:03 ` [lvc-project] " Alexey Khoroshilov
2023-03-14 22:04 ` Linus Walleij
1 sibling, 1 reply; 11+ messages in thread
From: Andi Shyti @ 2023-03-14 21:28 UTC (permalink / raw)
To: Linus Walleij
Cc: Kasumov Ruslan, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Jonathan Cameron, Lars-Peter Clausen, linux-arm-msm, linux-iio,
linux-kernel, lvc-project, Kasumov Ruslan
Hi,
> > The left side of the loop condition never becomes false.
> > hwchan cannot be NULL, because it points to elements of the
> > hw_channels array that takes one of 4 predefined values:
> > pm8018_xoadc_channels, pm8038_xoadc_channels,
> > pm8058_xoadc_channels, pm8921_xoadc_channels.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> I am not impressed with that tool. See below:
>
> > Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
> > Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
>
> (...)
> > hwchan = &hw_channels[0];
> > - while (hwchan && hwchan->datasheet_name) {
> > + while (hwchan->datasheet_name) {
> > if (hwchan->pre_scale_mux == pre_scale_mux &&
> > hwchan->amux_channel == amux_channel)
> > break;
>
> NAK have you tested this on a real system?
>
> Here is the complete loop:
>
> hwchan = &hw_channels[0];
> while (hwchan && hwchan->datasheet_name) {
> if (hwchan->pre_scale_mux == pre_scale_mux &&
> hwchan->amux_channel == amux_channel)
> break;
> hwchan++;
ops, yes, sorry, I overlooked at this... This becomes NULL at
some point as there is a sentinel in the _variant structures.
Thanks,
Andi
> chid++;
> }
>
> Notice how hwchan is used as iterator in hwchan++.
>
> What you are doing will cause a zero-pointer dereference.
>
> Whoever is developing "SVACE" needs to fix the tool to understand
> this kind of usage.
>
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [lvc-project] [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 21:28 ` Andi Shyti
@ 2023-03-14 22:03 ` Alexey Khoroshilov
2023-03-14 22:07 ` Linus Walleij
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Khoroshilov @ 2023-03-14 22:03 UTC (permalink / raw)
To: Andi Shyti, Linus Walleij
Cc: Lars-Peter Clausen, lvc-project, Kasumov Ruslan, linux-arm-msm,
Bjorn Andersson, linux-kernel, Konrad Dybcio, linux-iio,
Andy Gross, Kasumov Ruslan, Jonathan Cameron
On 15.03.2023 00:28, Andi Shyti wrote:
> Hi,
>
>>> The left side of the loop condition never becomes false.
>>> hwchan cannot be NULL, because it points to elements of the
>>> hw_channels array that takes one of 4 predefined values:
>>> pm8018_xoadc_channels, pm8038_xoadc_channels,
>>> pm8058_xoadc_channels, pm8921_xoadc_channels.
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> I am not impressed with that tool. See below:
>>
>>> Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
>>> Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
>>
>> (...)
>>> hwchan = &hw_channels[0];
>>> - while (hwchan && hwchan->datasheet_name) {
>>> + while (hwchan->datasheet_name) {
>>> if (hwchan->pre_scale_mux == pre_scale_mux &&
>>> hwchan->amux_channel == amux_channel)
>>> break;
>>
>> NAK have you tested this on a real system?
>>
>> Here is the complete loop:
>>
>> hwchan = &hw_channels[0];
>> while (hwchan && hwchan->datasheet_name) {
>> if (hwchan->pre_scale_mux == pre_scale_mux &&
>> hwchan->amux_channel == amux_channel)
>> break;
>> hwchan++;
>
> ops, yes, sorry, I overlooked at this... This becomes NULL at
> some point as there is a sentinel in the _variant structures.
>
Could you please clarify what do you mean.
As far as I can see sentinel is an "empty" element of xoadc_channel in
the array, i.e. hwchan->datasheet_name works as a sentinel while hwchan
is always non NULL.
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 21:12 ` Linus Walleij
2023-03-14 21:28 ` Andi Shyti
@ 2023-03-14 22:04 ` Linus Walleij
1 sibling, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2023-03-14 22:04 UTC (permalink / raw)
To: Kasumov Ruslan
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, linux-arm-msm, linux-iio, linux-kernel,
lvc-project, Kasumov Ruslan
On Tue, Mar 14, 2023 at 10:12 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Mar 14, 2023 at 8:37 PM Kasumov Ruslan <xhxgldhlpfy@gmail.com> wrote:
>
> > The left side of the loop condition never becomes false.
> > hwchan cannot be NULL, because it points to elements of the
> > hw_channels array that takes one of 4 predefined values:
> > pm8018_xoadc_channels, pm8038_xoadc_channels,
> > pm8058_xoadc_channels, pm8921_xoadc_channels.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> I am not impressed with that tool. See below:
>
> > Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
> > Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
>
> (...)
> > hwchan = &hw_channels[0];
> > - while (hwchan && hwchan->datasheet_name) {
> > + while (hwchan->datasheet_name) {
> > if (hwchan->pre_scale_mux == pre_scale_mux &&
> > hwchan->amux_channel == amux_channel)
> > break;
>
> NAK have you tested this on a real system?
>
> Here is the complete loop:
>
> hwchan = &hw_channels[0];
> while (hwchan && hwchan->datasheet_name) {
> if (hwchan->pre_scale_mux == pre_scale_mux &&
> hwchan->amux_channel == amux_channel)
> break;
> hwchan++;
> chid++;
> }
>
> Notice how hwchan is used as iterator in hwchan++.
>
> What you are doing will cause a zero-pointer dereference.
Nah the AI is smarter than me this time, I'm wrong, I think :(
hwchan is indeed never NULL here, and the code immediately
after unconditionally dereferences hwchan->datasheet_name.
Who wrote this convoluted code again:
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 759)
chid = 0;
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 760)
hwchan = &hw_channels[0];
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 761)
while (hwchan && hwchan->datasheet_name) {
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 762)
if (hwchan->pre_scale_mux == pre_scale_mux &&
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 763)
hwchan->amux_channel == amux_channel)
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 764)
break;
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 765)
hwchan++;
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 766)
chid++;
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 767) }
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 768)
/* The sentinel does not have a name assigned */
63c3ecd946d4a (Linus Walleij 2017-04-04 14:08:19 +0200 769)
if (!hwchan->datasheet_name) {
Oh that guy ...
I wonder if we can make it look better and less unintuitive.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [lvc-project] [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 22:03 ` [lvc-project] " Alexey Khoroshilov
@ 2023-03-14 22:07 ` Linus Walleij
2023-03-14 22:41 ` Andi Shyti
0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2023-03-14 22:07 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Andi Shyti, Lars-Peter Clausen, lvc-project, Kasumov Ruslan,
linux-arm-msm, Bjorn Andersson, linux-kernel, Konrad Dybcio,
linux-iio, Andy Gross, Kasumov Ruslan, Jonathan Cameron
On Tue, Mar 14, 2023 at 11:03 PM Alexey Khoroshilov
<khoroshilov@ispras.ru> wrote:
> As far as I can see sentinel is an "empty" element of xoadc_channel in
> the array, i.e. hwchan->datasheet_name works as a sentinel while hwchan
> is always non NULL.
You're right, I was unable to understand my own code :(
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 19:37 [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel() Kasumov Ruslan
2023-03-14 20:42 ` Andi Shyti
2023-03-14 21:12 ` Linus Walleij
@ 2023-03-14 22:16 ` Linus Walleij
2023-03-15 13:51 ` [PATCH v2] " Kasumov Ruslan
3 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2023-03-14 22:16 UTC (permalink / raw)
To: Kasumov Ruslan
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, linux-arm-msm, linux-iio, linux-kernel,
lvc-project, Kasumov Ruslan
On Tue, Mar 14, 2023 at 8:37 PM Kasumov Ruslan <xhxgldhlpfy@gmail.com> wrote:
> The left side of the loop condition never becomes false.
> hwchan cannot be NULL, because it points to elements of the
> hw_channels array that takes one of 4 predefined values:
> pm8018_xoadc_channels, pm8038_xoadc_channels,
> pm8058_xoadc_channels, pm8921_xoadc_channels.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 63c3ecd946d4 ("iio: adc: add a driver for Qualcomm PM8xxx HK/XOADC")
> Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [lvc-project] [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 22:07 ` Linus Walleij
@ 2023-03-14 22:41 ` Andi Shyti
0 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-03-14 22:41 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexey Khoroshilov, Andi Shyti, Lars-Peter Clausen, lvc-project,
Kasumov Ruslan, linux-arm-msm, Bjorn Andersson, linux-kernel,
Konrad Dybcio, linux-iio, Andy Gross, Kasumov Ruslan,
Jonathan Cameron
Hi Alexey and Ruslan,
On Tue, Mar 14, 2023 at 11:07:19PM +0100, Linus Walleij wrote:
> On Tue, Mar 14, 2023 at 11:03 PM Alexey Khoroshilov
> <khoroshilov@ispras.ru> wrote:
>
> > As far as I can see sentinel is an "empty" element of xoadc_channel in
> > the array, i.e. hwchan->datasheet_name works as a sentinel while hwchan
> > is always non NULL.
>
> You're right, I was unable to understand my own code :(
At this time of the day I got alarmed too. Happens :)
Please ignore my previous comment but still no need for the
Fixes: tag from the commit log as it's a cleanup and not a bug
fix.
Thanks,
Andi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-14 19:37 [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel() Kasumov Ruslan
` (2 preceding siblings ...)
2023-03-14 22:16 ` Linus Walleij
@ 2023-03-15 13:51 ` Kasumov Ruslan
2023-03-18 15:48 ` Jonathan Cameron
3 siblings, 1 reply; 11+ messages in thread
From: Kasumov Ruslan @ 2023-03-15 13:51 UTC (permalink / raw)
To: Andy Gross
Cc: Bjorn Andersson, Konrad Dybcio, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij, Andi Shyti, linux-arm-msm,
linux-iio, linux-kernel, lvc-project, Kasumov Ruslan
The left side of the loop condition never becomes false.
hwchan cannot be NULL, because it points to elements of the
hw_channels array that takes one of 4 predefined values:
pm8018_xoadc_channels, pm8038_xoadc_channels,
pm8058_xoadc_channels, pm8921_xoadc_channels.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
---
v2: Removed "Fixes" tag as Andi Shyti <andi.shyti@kernel.org> suggested.
drivers/iio/adc/qcom-pm8xxx-xoadc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index eb424496ee1d..64a3aeb6261c 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -758,7 +758,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
/* Find the right channel setting */
chid = 0;
hwchan = &hw_channels[0];
- while (hwchan && hwchan->datasheet_name) {
+ while (hwchan->datasheet_name) {
if (hwchan->pre_scale_mux == pre_scale_mux &&
hwchan->amux_channel == amux_channel)
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel()
2023-03-15 13:51 ` [PATCH v2] " Kasumov Ruslan
@ 2023-03-18 15:48 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2023-03-18 15:48 UTC (permalink / raw)
To: Kasumov Ruslan
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Lars-Peter Clausen,
Linus Walleij, Andi Shyti, linux-arm-msm, linux-iio,
linux-kernel, lvc-project, Kasumov Ruslan
On Wed, 15 Mar 2023 16:51:14 +0300
Kasumov Ruslan <xhxgldhlpfy@gmail.com> wrote:
> The left side of the loop condition never becomes false.
> hwchan cannot be NULL, because it points to elements of the
> hw_channels array that takes one of 4 predefined values:
> pm8018_xoadc_channels, pm8038_xoadc_channels,
> pm8058_xoadc_channels, pm8921_xoadc_channels.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Kasumov Ruslan <s02210418@gse.cs.msu.ru>
We could file this under the category of (overly) paranoid checking that
he variant structure has the hw_channels set, but given there are only 4 instances
of that it is (as you have done) easy to check.
So fair enough to drop this check.
Applied.
Thanks,
Jonathan
> ---
> v2: Removed "Fixes" tag as Andi Shyti <andi.shyti@kernel.org> suggested.
> drivers/iio/adc/qcom-pm8xxx-xoadc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> index eb424496ee1d..64a3aeb6261c 100644
> --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> @@ -758,7 +758,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
> /* Find the right channel setting */
> chid = 0;
> hwchan = &hw_channels[0];
> - while (hwchan && hwchan->datasheet_name) {
> + while (hwchan->datasheet_name) {
> if (hwchan->pre_scale_mux == pre_scale_mux &&
> hwchan->amux_channel == amux_channel)
> break;
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-03-18 15:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-14 19:37 [PATCH] iio: adc: qcom-pm8xxx-xoadc: Remove useless condition in pm8xxx_xoadc_parse_channel() Kasumov Ruslan
2023-03-14 20:42 ` Andi Shyti
2023-03-14 21:12 ` Linus Walleij
2023-03-14 21:28 ` Andi Shyti
2023-03-14 22:03 ` [lvc-project] " Alexey Khoroshilov
2023-03-14 22:07 ` Linus Walleij
2023-03-14 22:41 ` Andi Shyti
2023-03-14 22:04 ` Linus Walleij
2023-03-14 22:16 ` Linus Walleij
2023-03-15 13:51 ` [PATCH v2] " Kasumov Ruslan
2023-03-18 15:48 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.