Re: [PATCH v1 2/9] pinctrl: tegra30: fix "Possible null pointer dereference: group" warning

[Lucas Stach <l.stach@pengutronix.de>]

Am Dienstag, den 20.11.2018, 21:07 +0100 schrieb Oleksij Rempel:
> The code is correct but it takes more seconds for me to understand.
> And static code analyzer do not understand it at all.
> 
> > Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
> ---
>  drivers/pinctrl/pinctrl-tegra30.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-tegra30.c b/drivers/pinctrl/pinctrl-tegra30.c
> index d9b49c57d..ffb04eebb 100644
> --- a/drivers/pinctrl/pinctrl-tegra30.c
> +++ b/drivers/pinctrl/pinctrl-tegra30.c
> @@ -658,8 +658,8 @@ static int pinctrl_tegra30_set_drvstate(struct pinctrl_tegra30 *ctrl,
> >  			break;
> >  		}
> >  	}
> > -	/* if no matching drivegroup is found */
> > -	if (i == ctrl->drvdata->num_drvgrps)
> +
> > +	if (!group)
> >  		return 0;

Huh? This is a pretty standard idiom in C codebases to check if we
broke out of a loop early.

Actually this change breaks the code, as this check is inside of an
outer loop that doesn't reinitialize the group variable. So while the
code as-is correctly checks if a group was found in the current
iteration of the outer loop, after this patch it also matches a group
that was found on a previous iteration of the outer loop.

This is a prime example where static checker warnings can prompt wrong
fixes. Frankly codacy should smart up to correctly analyze the
controlflow interdependency.

Sascha, please drop this patch.

Regards,
Lucas

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox