* [PATCH] gpio: mpc8xxx: resolve coverity warnings
@ 2020-12-03 7:39 Biwen Li
2020-12-03 8:01 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Biwen Li @ 2020-12-03 7:39 UTC (permalink / raw)
To: leoyang.li, bgolaszewski, aisheng.dong
Cc: linux-kernel, jiafei.pan, linux-gpio, Biwen Li
From: Biwen Li <biwen.li@nxp.com>
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@nxp.com>
---
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@sunsite.dk>
* Copyright (C) 2016 Freescale Semiconductor Inc.
+ * Copyright 2020 NXP
*
* 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;
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gpio: mpc8xxx: resolve coverity warnings
2020-12-03 7:39 [PATCH] gpio: mpc8xxx: resolve coverity warnings Biwen Li
@ 2020-12-03 8:01 ` Bartosz Golaszewski
2020-12-03 8:07 ` Biwen Li (OSS)
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2020-12-03 8:01 UTC (permalink / raw)
To: Biwen Li; +Cc: Li Yang, aisheng.dong, LKML, jiafei.pan, linux-gpio, Biwen Li
On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
>
> From: Biwen Li <biwen.li@nxp.com>
>
> 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@nxp.com>
> ---
> 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@sunsite.dk>
> * 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?
> *
> * 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?
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] gpio: mpc8xxx: resolve coverity warnings
2020-12-03 8:01 ` Bartosz Golaszewski
@ 2020-12-03 8:07 ` Biwen Li (OSS)
2020-12-03 8:18 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Biwen Li (OSS) @ 2020-12-03 8:07 UTC (permalink / raw)
To: Bartosz Golaszewski, Biwen Li (OSS)
Cc: Leo Li, Aisheng Dong, LKML, Jiafei Pan, linux-gpio
> On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
> >
> > From: Biwen Li <biwen.li@nxp.com>
> >
> > 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@nxp.com>
> > ---
> > 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@sunsite.dk>
> > * 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)
>
> 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
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpio: mpc8xxx: resolve coverity warnings
2020-12-03 8:07 ` Biwen Li (OSS)
@ 2020-12-03 8:18 ` Bartosz Golaszewski
2020-12-03 8:48 ` Biwen Li (OSS)
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2020-12-03 8:18 UTC (permalink / raw)
To: Biwen Li (OSS); +Cc: Leo Li, Aisheng Dong, LKML, Jiafei Pan, linux-gpio
On Thu, Dec 3, 2020 at 9:07 AM Biwen Li (OSS) <biwen.li@oss.nxp.com> wrote:
>
> > On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
> > >
> > > From: Biwen Li <biwen.li@nxp.com>
> > >
> > > 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@nxp.com>
> > > ---
> > > 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@sunsite.dk>
> > > * 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
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] gpio: mpc8xxx: resolve coverity warnings
2020-12-03 8:18 ` Bartosz Golaszewski
@ 2020-12-03 8:48 ` Biwen Li (OSS)
2020-12-03 9:03 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Biwen Li (OSS) @ 2020-12-03 8:48 UTC (permalink / raw)
To: Bartosz Golaszewski, Biwen Li (OSS)
Cc: Leo Li, Aisheng Dong, LKML, Jiafei Pan, linux-gpio
>
> On Thu, Dec 3, 2020 at 9:07 AM Biwen Li (OSS) <biwen.li@oss.nxp.com> wrote:
> >
> > > On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
> > > >
> > > > From: Biwen Li <biwen.li@nxp.com>
> > > >
> > > > 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@nxp.com>
> > > > ---
> > > > 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@sunsite.dk>
> > > > * 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
> > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpio: mpc8xxx: resolve coverity warnings
2020-12-03 8:48 ` Biwen Li (OSS)
@ 2020-12-03 9:03 ` Bartosz Golaszewski
0 siblings, 0 replies; 6+ messages in thread
From: Bartosz Golaszewski @ 2020-12-03 9:03 UTC (permalink / raw)
To: Biwen Li (OSS)
Cc: Bartosz Golaszewski, Leo Li, Aisheng Dong, LKML, Jiafei Pan, linux-gpio
On Thu, Dec 3, 2020 at 9:51 AM Biwen Li (OSS) <biwen.li@oss.nxp.com> wrote:
>
> >
> > On Thu, Dec 3, 2020 at 9:07 AM Biwen Li (OSS) <biwen.li@oss.nxp.com> wrote:
> > >
> > > > On Thu, Dec 3, 2020 at 8:31 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
> > > > >
> > > > > From: Biwen Li <biwen.li@nxp.com>
> > > > >
> > > > > 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@nxp.com>
> > > > > ---
> > > > > 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@sunsite.dk>
> > > > > * 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.
> >
This only makes sense if we're using large values that could
potentially overflow a signed int. This is a small constant value so
there's no reason for this churn. NAK
Bartosz
> > 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
> > > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-03 9:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 7:39 [PATCH] gpio: mpc8xxx: resolve coverity warnings Biwen Li
2020-12-03 8:01 ` Bartosz Golaszewski
2020-12-03 8:07 ` Biwen Li (OSS)
2020-12-03 8:18 ` Bartosz Golaszewski
2020-12-03 8:48 ` Biwen Li (OSS)
2020-12-03 9:03 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).