[PATCH] pinctrl: sh-pfc: Make legacy function GPIO handling less fragile

[PATCH] pinctrl: sh-pfc: Make legacy function GPIO handling less fragile

[Geert Uytterhoeven]

If there are no function GPIOs, sh_pfc_register_gpiochip() returns early
with a success indicator.  This is fragile, as new code may be added
after the #ifdef block, which won't be executed in case of early return.

Invert the logic, so the code always continues until the end of the
function on success.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
To be queued in sh-pfc-for-v5.6.

 drivers/pinctrl/sh-pfc/gpio.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/gpio.c b/drivers/pinctrl/sh-pfc/gpio.c
index 5a55b8da7919520e..8213e118aa408573 100644
--- a/drivers/pinctrl/sh-pfc/gpio.c
+++ b/drivers/pinctrl/sh-pfc/gpio.c
@@ -386,12 +386,11 @@ int sh_pfc_register_gpiochip(struct sh_pfc *pfc)
 	}
 
 	/* Register the function GPIOs chip. */
-	if (pfc->info->nr_func_gpios == 0)
-		return 0;
-
-	chip = sh_pfc_add_gpiochip(pfc, gpio_function_setup, NULL);
-	if (IS_ERR(chip))
-		return PTR_ERR(chip);
+	if (pfc->info->nr_func_gpios) {
+		chip = sh_pfc_add_gpiochip(pfc, gpio_function_setup, NULL);
+		if (IS_ERR(chip))
+			return PTR_ERR(chip);
+	}
 #endif /* CONFIG_PINCTRL_SH_FUNC_GPIO */
 
 	return 0;
-- 
2.17.1

Re: [PATCH] pinctrl: sh-pfc: Make legacy function GPIO handling less fragile

[Niklas Söderlund]

Hi Geert,

Thanks for your work.

On 2019-11-13 11:18:09 +0100, Geert Uytterhoeven wrote:
> If there are no function GPIOs, sh_pfc_register_gpiochip() returns early
> with a success indicator.  This is fragile, as new code may be added
> after the #ifdef block, which won't be executed in case of early return.
> 
> Invert the logic, so the code always continues until the end of the
> function on success.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> To be queued in sh-pfc-for-v5.6.
> 
>  drivers/pinctrl/sh-pfc/gpio.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/gpio.c b/drivers/pinctrl/sh-pfc/gpio.c
> index 5a55b8da7919520e..8213e118aa408573 100644
> --- a/drivers/pinctrl/sh-pfc/gpio.c
> +++ b/drivers/pinctrl/sh-pfc/gpio.c
> @@ -386,12 +386,11 @@ int sh_pfc_register_gpiochip(struct sh_pfc *pfc)
>  	}
>  
>  	/* Register the function GPIOs chip. */
> -	if (pfc->info->nr_func_gpios == 0)
> -		return 0;
> -
> -	chip = sh_pfc_add_gpiochip(pfc, gpio_function_setup, NULL);
> -	if (IS_ERR(chip))
> -		return PTR_ERR(chip);
> +	if (pfc->info->nr_func_gpios) {
> +		chip = sh_pfc_add_gpiochip(pfc, gpio_function_setup, NULL);
> +		if (IS_ERR(chip))
> +			return PTR_ERR(chip);
> +	}
>  #endif /* CONFIG_PINCTRL_SH_FUNC_GPIO */
>  
>  	return 0;
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund