Re: [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions

[Simon Horman <horms@xxxxxxxxxxxx>]

On Thu, Apr 25, 2019 at 11:55:41AM +0200, Geert Uytterhoeven wrote:
> The pinmux groups and functions arrays may contain two parts, to ease
> supporting SoCs that expose pin subsets of other related SoCs.  Both
> parts need to be declared with explicit sizes, which thus need to be
> updated when adding support for more groups and functions.
> 
> If a size is too small, the compiler will detect this at build time
> ("excess elements in array initializer").
> If a size is too large, this may go undetected (for pin groups), lead to
> pin controller registration failures (for pin functions: "pinmux ops has
> no name for functionN"), or crash the optional run-time debug code (for
> pin groups).
> 
> Extend the run-time debug code with checks to detect this, to help
> catching bugs early.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Reviewed-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>

> ---
>  drivers/pinctrl/sh-pfc/core.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 868b9551438efb0a..889e5af281022e1d 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  
>  	for (i = 0; i < info->nr_functions; i++) {
>  		func = &info->functions[i];
> +		if (!func->name) {
> +			pr_err("%s: empty function %u\n", drvname, i);
> +			sh_pfc_errors++;

Not strictly related to this patch but did
you consider not having sh_pfc_errors global to this file?

> +			continue;
> +		}
>  		for (j = 0; j < func->nr_groups; j++) {
>  			for (k = 0; k < info->nr_groups; k++) {
> -				if (!strcmp(func->groups[j],
> +				if (info->groups[k].name &&
> +				    !strcmp(func->groups[j],
>  					    info->groups[k].name)) {
>  					refcnts[k]++;
>  					break;
> @@ -798,6 +804,11 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  	}
>  
>  	for (i = 0; i < info->nr_groups; i++) {
> +		if (!info->groups[i].name) {
> +			pr_err("%s: empty group %u\n", drvname, i);
> +			sh_pfc_errors++;
> +			continue;
> +		}
>  		if (!refcnts[i]) {
>  			pr_err("%s: orphan group %s\n", drvname,
>  			       info->groups[i].name);
> -- 
> 2.17.1
>