Re: [PATCH] gpio: mpc8xxx: resolve coverity warnings

[Bartosz Golaszewski]

On Thu, Dec 3, 2020 at 9:07 AM Biwen Li (OSS) <biwen.li@xxxxxxxxxxx> wrote:
>
> > On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@xxxxxxxxxxx> wrote:
> > >
> > > From: Biwen Li <biwen.li@xxxxxxx>
> > >
> > > Resolve coverity warnings as follows,
> > >     cond_at_most: Checking gpio >= 28U implies that gpio may be up
> > >     to 27 on the false branch.
> > >     overrun-call: Overrunning callees array of size 3 by passing
> > >     argument gpio (which evaluates to 27)
> > >     in call to *mpc8xxx_gc->direction_output
> > >
> > >     cond_at_least: Checking gpio <= 3U implies that gpio is at least 4 on
> > >     the false branch.
> > >     overrun-call: Overrunning callee's array of size 3 by passing argument
> > >     gpio (which evaluates to 4) in call to
> > > *mpc8xxx_gc->direction_output
> > >
> > > Signed-off-by: Biwen Li <biwen.li@xxxxxxx>
> > > ---
> > >  drivers/gpio/gpio-mpc8xxx.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > > index a6c2bbdcaa10..12c9a91d87b7 100644
> > > --- a/drivers/gpio/gpio-mpc8xxx.c
> > > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > > @@ -3,6 +3,7 @@
> > >   *
> > >   * Copyright (C) 2008 Peter Korsgaard <jacmet@xxxxxxxxxx>
> > >   * Copyright (C) 2016 Freescale Semiconductor Inc.
> > > + * Copyright 2020 NXP
> >
> > A copyright notice on a two-line change is a bit too much, don't you think?
> Okay, got it. Will remove it in v2.
> >
> > >   *
> > >   * This file is licensed under the terms of the GNU General Public License
> > >   * version 2.  This program is licensed "as is" without any warranty
> > > of any @@ -80,7 +81,7 @@ static int mpc5121_gpio_dir_out(struct
> > > gpio_chip *gc,  {
> > >         struct mpc8xxx_gpio_chip *mpc8xxx_gc = gpiochip_get_data(gc);
> > >         /* GPIO 28..31 are input only on MPC5121 */
> > > -       if (gpio >= 28)
> > > +       if (gpio >= 28U)
> > >                 return -EINVAL;
> >
> > I don't really understand the commit message but looking at the code is even
> > more confusing. What are you fixing here actually?
> Try to fix code warning that generated by coverity scan tool(static code analysis tool)

Please explain what benefit there is to using 28U over 28. No tool is
perfect, that's why you should try to understand what it is it's
trying to fix. I don't see any reason this code could fail.

Bartosz

> >
> > Bartosz
> >
> > >
> > >         return mpc8xxx_gc->direction_output(gc, gpio, val); @@ -91,7
> > > +92,7 @@ static int mpc5125_gpio_dir_out(struct gpio_chip *gc,  {
> > >         struct mpc8xxx_gpio_chip *mpc8xxx_gc = gpiochip_get_data(gc);
> > >         /* GPIO 0..3 are input only on MPC5125 */
> > > -       if (gpio <= 3)
> > > +       if (gpio <= 3U)
> > >                 return -EINVAL;
> > >
> > >         return mpc8xxx_gc->direction_output(gc, gpio, val);
> > > --
> > > 2.17.1
> > >