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 <[email protected]>
> ---
>  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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to