Re: [PATCH 2/2] powercap: arm_scmi: Fix a NULL vs IS_ERR() bug

[Cristian Marussi <cristian.marussi@arm.com>]

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
>