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

[Biwen Li (OSS)]

> 
> 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.
This code couldn't fail.
The variable gpio is unsigned int type, prefer to append "U" for unsigned typed values, this makes is clearer also when comparing values and variables.
> 
> 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
> > > >