1. Patchew
  2. linux
  3. [PATCH 0/2] regulator: s2mps11: two small bug fixes (s2mpg1x)
  4. View patch

[PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

André Draszik posted 2 patches 1 day, 4 hours ago
There is a newer version of this series
[PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by André Draszik 1 day, 4 hours ago 
The sanity checks being removed in this commit are useless as earlier
code checks for out-of-bounds conditions already. They also are
incorrect (as they're off-by-one).

Simply remove this incorrect code.

No functional change.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aYmsu8qREppwBESH@stanley.mountain/
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/s2mps11.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 2d5510acd0780ab6f9296c48ddcde5efe15ff488..2d67c5c16f487506a2e9e4b119f33faa846269f7 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -478,8 +478,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg10))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg10[ext_control];
 		break;
 
@@ -503,8 +501,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg11))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg11[ext_control];
 		break;
 

-- 
2.53.0.rc2.204.g2597b5adb4-goog
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by Krzysztof Kozlowski 1 day, 3 hours ago 
On 09/02/2026 16:07, André Draszik wrote:
> The sanity checks being removed in this commit are useless as earlier
> code checks for out-of-bounds conditions already. They also are
> incorrect (as they're off-by-one).
> 
> Simply remove this incorrect code.
> 
> No functional change.

If they are incorrect then how it could be "no functional change"? To me
original code looks buggy and this is a fix. Fix must have functional
change...

> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aYmsu8qREppwBESH@stanley.mountain/
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
Best regards,
Krzysztof
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by André Draszik 13 hours ago 
Hi Krzysztof,

On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
> On 09/02/2026 16:07, André Draszik wrote:
> > The sanity checks being removed in this commit are useless as earlier
> > code checks for out-of-bounds conditions already. They also are
> > incorrect (as they're off-by-one).
> > 
> > Simply remove this incorrect code.
> > 
> > No functional change.
> 
> If they are incorrect then how it could be "no functional change"? To me
> original code looks buggy and this is a fix. Fix must have functional
> change...

Earlier code already checks for all conditions, including all error cases.
So the code being removed here has no effect, as any potential error it
could catch will already have been caught by earlier code. Removing it
therefore doesn't change behaviour or functionality.

I can reword to 'incomplete test' instead of 'incorrect code' if you think
that's more clear?

Cheers,
Andre'
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by Krzysztof Kozlowski 11 hours ago 
On 10/02/2026 06:59, André Draszik wrote:
> Hi Krzysztof,
> 
> On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
>> On 09/02/2026 16:07, André Draszik wrote:
>>> The sanity checks being removed in this commit are useless as earlier
>>> code checks for out-of-bounds conditions already. They also are
>>> incorrect (as they're off-by-one).
>>>
>>> Simply remove this incorrect code.
>>>
>>> No functional change.
>>
>> If they are incorrect then how it could be "no functional change"? To me
>> original code looks buggy and this is a fix. Fix must have functional
>> change...
> 
> Earlier code already checks for all conditions, including all error cases.
> So the code being removed here has no effect, as any potential error it
> could catch will already have been caught by earlier code. Removing it
> therefore doesn't change behaviour or functionality.
> 
> I can reword to 'incomplete test' instead of 'incorrect code' if you think
> that's more clear?

Perhaps you should mention which "one" in off-by-one that it has no impact.

I also wonder why you left the second - ext_control_s2mpg11 - untouched.

Best regards,
Krzysztof
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by André Draszik 9 hours ago 
Hi Krzysztof,

On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
> On 10/02/2026 06:59, André Draszik wrote:
> > Hi Krzysztof,
> > 
> > On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
> > > On 09/02/2026 16:07, André Draszik wrote:
> > > > The sanity checks being removed in this commit are useless as earlier
> > > > code checks for out-of-bounds conditions already. They also are
> > > > incorrect (as they're off-by-one).
> > > > 
> > > > Simply remove this incorrect code.
> > > > 
> > > > No functional change.
> > > 
> > > If they are incorrect then how it could be "no functional change"? To me
> > > original code looks buggy and this is a fix. Fix must have functional
> > > change...
> > 
> > Earlier code already checks for all conditions, including all error cases.
> > So the code being removed here has no effect, as any potential error it
> > could catch will already have been caught by earlier code. Removing it
> > therefore doesn't change behaviour or functionality.
> > 
> > I can reword to 'incomplete test' instead of 'incorrect code' if you think
> > that's more clear?
> 
> Perhaps you should mention which "one" in off-by-one that it has no impact.

OK.

> I also wonder why you left the second - ext_control_s2mpg11 - untouched.

Could you point me to it please? I'm not sure I see what you mean.

Cheers,
Andre'
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by Krzysztof Kozlowski 8 hours ago 
On 10/02/2026 10:37, André Draszik wrote:
> Hi Krzysztof,
> 
> On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
>> On 10/02/2026 06:59, André Draszik wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
>>>> On 09/02/2026 16:07, André Draszik wrote:
>>>>> The sanity checks being removed in this commit are useless as earlier
>>>>> code checks for out-of-bounds conditions already. They also are
>>>>> incorrect (as they're off-by-one).
>>>>>
>>>>> Simply remove this incorrect code.
>>>>>
>>>>> No functional change.
>>>>
>>>> If they are incorrect then how it could be "no functional change"? To me
>>>> original code looks buggy and this is a fix. Fix must have functional
>>>> change...
>>>
>>> Earlier code already checks for all conditions, including all error cases.
>>> So the code being removed here has no effect, as any potential error it
>>> could catch will already have been caught by earlier code. Removing it
>>> therefore doesn't change behaviour or functionality.
>>>
>>> I can reword to 'incomplete test' instead of 'incorrect code' if you think
>>> that's more clear?
>>
>> Perhaps you should mention which "one" in off-by-one that it has no impact.
> 
> OK.
> 
>> I also wonder why you left the second - ext_control_s2mpg11 - untouched.
> 
> Could you point me to it please? I'm not sure I see what you mean.

There is exact same line, which you touch here, ~20 lines below. At
least in linux-next from 5th Feb.

Best regards,
Krzysztof
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by André Draszik 7 hours ago 
On Tue, 2026-02-10 at 11:51 +0100, Krzysztof Kozlowski wrote:
> On 10/02/2026 10:37, André Draszik wrote:
> > Hi Krzysztof,
> > 
> > On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
> > 
> > 
> > > I also wonder why you left the second - ext_control_s2mpg11 - untouched.
> > 
> > Could you point me to it please? I'm not sure I see what you mean.
> 
> There is exact same line, which you touch here, ~20 lines below. At
> least in linux-next from 5th Feb.

So you mean
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/s2mps11.c#n506
? It is part of the patch:

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 2d5510acd0780ab6f9296c48ddcde5efe15ff488..2d67c5c16f487506a2e9e4b119f33faa846269f7 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -478,8 +478,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg10))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg10[ext_control];
 		break;
 
@@ -503,8 +501,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg11))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg11[ext_control];
 		break;
 

Otherwise I still don't see it :-)


Cheers,
Andre'
Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
Posted by Krzysztof Kozlowski 7 hours ago 
On 10/02/2026 12:35, André Draszik wrote:
> On Tue, 2026-02-10 at 11:51 +0100, Krzysztof Kozlowski wrote:
>> On 10/02/2026 10:37, André Draszik wrote:
>>> Hi Krzysztof,
>>>
>>> On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
>>>
>>>
>>>> I also wonder why you left the second - ext_control_s2mpg11 - untouched.
>>>
>>> Could you point me to it please? I'm not sure I see what you mean.
>>
>> There is exact same line, which you touch here, ~20 lines below. At
>> least in linux-next from 5th Feb.
> 
> So you mean
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/s2mps11.c#n506
> ? It is part of the patch:
> 

D'oh!

With or without updates of commit msg:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.