* [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe
@ 2022-07-06 14:26 Dan Carpenter
2022-07-06 14:27 ` [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug Dan Carpenter
2022-07-06 14:52 ` [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Cristian Marussi
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-07-06 14:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Cristian Marussi
Cc: Sudeep Holla, linux-pm, kernel-janitors
The "pr->num_zones" variable is an unsigned int so it can't be less than
zero.
Fixes: 31afdd34f2b9 ("powercap: arm_scmi: Add SCMI powercap based driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/powercap/arm_scmi_powercap.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/powercap/arm_scmi_powercap.c b/drivers/powercap/arm_scmi_powercap.c
index 2273768d70ce..ab96cf9a8604 100644
--- a/drivers/powercap/arm_scmi_powercap.c
+++ b/drivers/powercap/arm_scmi_powercap.c
@@ -425,11 +425,12 @@ static int scmi_powercap_probe(struct scmi_device *sdev)
if (!pr)
return -ENOMEM;
- pr->num_zones = powercap_ops->num_domains_get(ph);
- if (pr->num_zones < 0) {
+ ret = powercap_ops->num_domains_get(ph);
+ if (ret < 0) {
dev_err(dev, "number of powercap domains not found\n");
- return pr->num_zones;
+ return ret;
}
+ pr->num_zones = ret;
pr->spzones = devm_kcalloc(dev, pr->num_zones,
sizeof(*pr->spzones), GFP_KERNEL);
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug
2022-07-06 14:26 [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Dan Carpenter
@ 2022-07-06 14:27 ` Dan Carpenter
2022-07-06 15:20 ` Cristian Marussi
2022-07-06 14:52 ` [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Cristian Marussi
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2022-07-06 14:27 UTC (permalink / raw)
To: Rafael J. Wysocki, Cristian Marussi
Cc: Sudeep Holla, linux-pm, kernel-janitors
The powercap_register_control_type() return error pointers. It never
returns NULL.
Fixes: 31afdd34f2b9 ("powercap: arm_scmi: Add SCMI powercap based driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This functions should really clean up after itself if scmi_register()
fails. I need to fix the static checker for that and then I'll come
back and fix it if no one else does.
drivers/powercap/arm_scmi_powercap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/powercap/arm_scmi_powercap.c b/drivers/powercap/arm_scmi_powercap.c
index ab96cf9a8604..2d505ec7ff81 100644
--- a/drivers/powercap/arm_scmi_powercap.c
+++ b/drivers/powercap/arm_scmi_powercap.c
@@ -519,8 +519,8 @@ static struct scmi_driver scmi_powercap_driver = {
static int __init scmi_powercap_init(void)
{
scmi_top_pcntrl = powercap_register_control_type(NULL, "arm-scmi", NULL);
- if (!scmi_top_pcntrl)
- return -ENODEV;
+ if (IS_ERR(scmi_top_pcntrl))
+ return PTR_ERR(scmi_top_pcntrl);
return scmi_register(&scmi_powercap_driver);
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug
2022-07-06 14:27 ` [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug Dan Carpenter
@ 2022-07-06 15:20 ` Cristian Marussi
2022-07-06 15:32 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Cristian Marussi @ 2022-07-06 15:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Rafael J. Wysocki, Sudeep Holla, linux-pm, kernel-janitors
On Wed, Jul 06, 2022 at 05:27:28PM +0300, Dan Carpenter wrote:
> The powercap_register_control_type() return error pointers. It never
> returns NULL.
>
> Fixes: 31afdd34f2b9 ("powercap: arm_scmi: Add SCMI powercap based driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This functions should really clean up after itself if scmi_register()
> fails. I need to fix the static checker for that and then I'll come
> back and fix it if no one else does.
>
Hi,
thanks for the fix and the suggestion to clean up better (this part was
indeed reworked in V4 and I think it's where I introduced the missing cleanup
when scmi_register fails...)
As said, the SCMI Powercap driver was NOT pulled for this cycle
due to insufficent reviews so I'll pick your fixes and suggestions
for the next version.
May I ask which static checker you use ? Sparse/smatch and W=1 did not
spot any of these issues (including other in the series) in my workflow ...
Thanks,
Cristian
> drivers/powercap/arm_scmi_powercap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/powercap/arm_scmi_powercap.c b/drivers/powercap/arm_scmi_powercap.c
> index ab96cf9a8604..2d505ec7ff81 100644
> --- a/drivers/powercap/arm_scmi_powercap.c
> +++ b/drivers/powercap/arm_scmi_powercap.c
> @@ -519,8 +519,8 @@ static struct scmi_driver scmi_powercap_driver = {
> static int __init scmi_powercap_init(void)
> {
> scmi_top_pcntrl = powercap_register_control_type(NULL, "arm-scmi", NULL);
> - if (!scmi_top_pcntrl)
> - return -ENODEV;
> + if (IS_ERR(scmi_top_pcntrl))
> + return PTR_ERR(scmi_top_pcntrl);
>
> return scmi_register(&scmi_powercap_driver);
> }
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug
2022-07-06 15:20 ` Cristian Marussi
@ 2022-07-06 15:32 ` Dan Carpenter
2022-07-06 15:46 ` Cristian Marussi
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2022-07-06 15:32 UTC (permalink / raw)
To: Cristian Marussi
Cc: Rafael J. Wysocki, Sudeep Holla, linux-pm, kernel-janitors
On Wed, Jul 06, 2022 at 04:20:41PM +0100, Cristian Marussi wrote:
> May I ask which static checker you use ? Sparse/smatch and W=1 did not
> spot any of these issues (including other in the series) in my workflow ...
>
These are Smatch warnings:
$ kchecker drivers/powercap/arm_scmi_powercap.c
Using test/ version of smatch
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CC [M] drivers/powercap/arm_scmi_powercap.o
CHECK drivers/powercap/arm_scmi_powercap.c
drivers/powercap/arm_scmi_powercap.c:429 scmi_powercap_probe() warn: unsigned 'pr->num_zones' is never less than zero.
drivers/powercap/arm_scmi_powercap.c:494 scmi_powercap_probe() error: uninitialized symbol 'ret'.
drivers/powercap/arm_scmi_powercap.c:521 scmi_powercap_init() warn: 'scmi_top_pcntrl' is an error pointer or valid
$
The problem is that the "is an error pointer or valid" requires the
cross function DB to work and that takes forever (over night on my
system).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug
2022-07-06 15:32 ` Dan Carpenter
@ 2022-07-06 15:46 ` Cristian Marussi
0 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2022-07-06 15:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Rafael J. Wysocki, Sudeep Holla, linux-pm, kernel-janitors
On Wed, Jul 06, 2022 at 06:32:33PM +0300, Dan Carpenter wrote:
> On Wed, Jul 06, 2022 at 04:20:41PM +0100, Cristian Marussi wrote:
> > May I ask which static checker you use ? Sparse/smatch and W=1 did not
> > spot any of these issues (including other in the series) in my workflow ...
> >
>
> These are Smatch warnings:
>
> $ kchecker drivers/powercap/arm_scmi_powercap.c
>
> Using test/ version of smatch
>
> CALL scripts/checksyscalls.sh
> CALL scripts/atomic/check-atomics.sh
> DESCEND objtool
> CC [M] drivers/powercap/arm_scmi_powercap.o
> CHECK drivers/powercap/arm_scmi_powercap.c
> drivers/powercap/arm_scmi_powercap.c:429 scmi_powercap_probe() warn: unsigned 'pr->num_zones' is never less than zero.
> drivers/powercap/arm_scmi_powercap.c:494 scmi_powercap_probe() error: uninitialized symbol 'ret'.
> drivers/powercap/arm_scmi_powercap.c:521 scmi_powercap_init() warn: 'scmi_top_pcntrl' is an error pointer or valid
> $
>
> The problem is that the "is an error pointer or valid" requires the
> cross function DB to work and that takes forever (over night on my
> system).
>
Thanks.
Turns out even my setup can spot it now (beside the last one), cause my workflow
self checks were targeted at where is usually rooted my work drivers/firmware/arm_scmi/
...so missing out completely on drivers/powercap ... my bad :<
Regards
Cristian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe
2022-07-06 14:26 [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Dan Carpenter
2022-07-06 14:27 ` [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug Dan Carpenter
@ 2022-07-06 14:52 ` Cristian Marussi
1 sibling, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2022-07-06 14:52 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Rafael J. Wysocki, Sudeep Holla, linux-pm, kernel-janitors
On Wed, Jul 06, 2022 at 05:26:56PM +0300, Dan Carpenter wrote:
> The "pr->num_zones" variable is an unsigned int so it can't be less than
> zero.
>
> Fixes: 31afdd34f2b9 ("powercap: arm_scmi: Add SCMI powercap based driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Hi,
Thanks for having a look.
This driver has NOT been pulled in this cycle due to a lack of
reviews/feedbacks so I'll integrate your fixes in the next version.
Thanks,
Cristian
> ---
> drivers/powercap/arm_scmi_powercap.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/powercap/arm_scmi_powercap.c b/drivers/powercap/arm_scmi_powercap.c
> index 2273768d70ce..ab96cf9a8604 100644
> --- a/drivers/powercap/arm_scmi_powercap.c
> +++ b/drivers/powercap/arm_scmi_powercap.c
> @@ -425,11 +425,12 @@ static int scmi_powercap_probe(struct scmi_device *sdev)
> if (!pr)
> return -ENOMEM;
>
> - pr->num_zones = powercap_ops->num_domains_get(ph);
> - if (pr->num_zones < 0) {
> + ret = powercap_ops->num_domains_get(ph);
> + if (ret < 0) {
> dev_err(dev, "number of powercap domains not found\n");
> - return pr->num_zones;
> + return ret;
> }
> + pr->num_zones = ret;
>
> pr->spzones = devm_kcalloc(dev, pr->num_zones,
> sizeof(*pr->spzones), GFP_KERNEL);
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-06 15:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 14:26 [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Dan Carpenter
2022-07-06 14:27 ` [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug Dan Carpenter
2022-07-06 15:20 ` Cristian Marussi
2022-07-06 15:32 ` Dan Carpenter
2022-07-06 15:46 ` Cristian Marussi
2022-07-06 14:52 ` [PATCH 1/2] powercap: arm_scmi: Fix signedness bug in probe Cristian Marussi
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.