[PATCH v1 0/9] random fixes

[PATCH v1 0/9] random fixes

[Oleksij Rempel]

next round of random fixes reported by codacy:
https://app.codacy.com/app/olerem/barebox/issues/index?bid=9803255

Oleksij Rempel (9):
  net: e1000: fix "Uninitialized variable: phy_data" warning
  pinctrl: tegra30: fix "Possible null pointer dereference: group"
    warning
  fs: closedir: remove uninitialized variable ret
  fs: fix possible null pointer dereference of base.
  mfd: da9063: fix wrong NULL pointer test
  usb: at91_udc: remove useless NULL check
  mtd: atmel_nand: remove erroneous case
  usb: musb: fix possible out of bounds access
  lib: lodepng: remove useless test

 drivers/mfd/da9063.c               | 2 +-
 drivers/mtd/nand/atmel_nand.c      | 1 -
 drivers/net/e1000/main.c           | 2 +-
 drivers/pinctrl/pinctrl-tegra30.c  | 4 ++--
 drivers/usb/gadget/at91_udc.c      | 3 +--
 drivers/usb/musb/musb_gadget_ep0.c | 7 ++++++-
 fs/fs.c                            | 7 +++----
 lib/gui/lodepng.c                  | 8 +-------
 8 files changed, 15 insertions(+), 19 deletions(-)

-- 
2.17.1


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

[PATCH v1 1/9] net: e1000: fix "Uninitialized variable: phy_data" warning

[Oleksij Rempel]

After carefully reading the code, this situation should
never happen. This patch is to reduce warning noise.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 drivers/net/e1000/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index bb6ab4eb0..caa7274a8 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -1130,7 +1130,7 @@ static int32_t e1000_set_d3_lplu_state_off(struct e1000_hw *hw)
 {
 	uint32_t phy_ctrl = 0;
 	int32_t ret_val;
-	uint16_t phy_data;
+	uint16_t phy_data = 0;
 	DEBUGFUNC();
 
 	/* During driver activity LPLU should not be used or it will attain link
-- 
2.17.1


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

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

[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;
 
 	regaddr = ctrl->regs.ctrl + (group->reg >> 2);
-- 
2.17.1


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

[PATCH v1 3/9] fs: closedir: remove uninitialized variable ret

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 fs/fs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index a30afa35f..42fd63ad0 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2527,8 +2527,6 @@ EXPORT_SYMBOL(opendir);
 
 int closedir(DIR *dir)
 {
-	int ret;
-
 	if (!dir) {
 		errno = EBADF;
 		return -EBADF;
@@ -2536,7 +2534,7 @@ int closedir(DIR *dir)
 
 	release_dir(dir);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(closedir);
 
-- 
2.17.1


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

[PATCH v1 4/9] fs: fix possible null pointer dereference of base.

[Oleksij Rempel]

we use base before checking if it is NULL

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 fs/fs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index 42fd63ad0..625ed10b7 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1526,7 +1526,7 @@ static struct dentry *__lookup_hash(const struct qstr *name,
 {
 	struct dentry *dentry;
 	struct dentry *old;
-	struct inode *dir = base->d_inode;
+	struct inode *dir;
 
 	if (!base)
 		return ERR_PTR(-ENOENT);
@@ -1539,6 +1539,7 @@ static struct dentry *__lookup_hash(const struct qstr *name,
 	if (unlikely(!dentry))
 		return ERR_PTR(-ENOMEM);
 
+	dir = base->d_inode;
 	old = dir->i_op->lookup(dir, dentry, flags);
 	if (IS_ERR(old)) {
 		dput(dentry);
-- 
2.17.1


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

[PATCH v1 5/9] mfd: da9063: fix wrong NULL pointer test

[Oleksij Rempel]

checking for priv == NULL and using in this case priv->dev
makes no sense.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 drivers/mfd/da9063.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
index b6114a614..a7fff2c5b 100644
--- a/drivers/mfd/da9063.c
+++ b/drivers/mfd/da9063.c
@@ -201,7 +201,7 @@ static int da9062_device_init(struct da9063 *priv)
 
 	priv->client1 = i2c_new_dummy(priv->client->adapter,
 				      priv->client->addr + 1);
-	if (!priv) {
+	if (!priv->client1) {
 		dev_warn(priv->dev, "failed to create bank 1 device\n");
 		/* TODO: return -EINVAL; i2c api does not return more
 		 * details */
-- 
2.17.1


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

[PATCH v1 6/9] usb: at91_udc: remove useless NULL check

[Oleksij Rempel]

if _ep or ep is NULL, we would get at least in two places
before this test a NULL pointer dereference.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 drivers/usb/gadget/at91_udc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 729f75212..243656d44 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -288,8 +288,7 @@ static int at91_ep_enable(struct usb_ep *_ep,
 	u16		maxpacket;
 	u32		tmp;
 
-	if (!_ep || !ep
-			|| !desc || ep->desc
+	if (!desc || ep->desc
 			|| _ep->name == ep0name
 			|| desc->bDescriptorType != USB_DT_ENDPOINT
 			|| (maxpacket = le16_to_cpu(desc->wMaxPacketSize)) == 0
-- 
2.17.1


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

[PATCH v1 7/9] mtd: atmel_nand: remove erroneous case

[Oleksij Rempel]

If mm would be 15, then we will corrupt the stack, since p[15]. And
as input for this function, mm has only two variants 13 and 14.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 drivers/mtd/nand/atmel_nand.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 797cdc2ba..4c1725d09 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -788,7 +788,6 @@ static int pmecc_build_galois_table(unsigned int mm, int16_t *index_of,
 	case 3:
 	case 4:
 	case 6:
-	case 15:
 		p[1] = 1;
 		break;
 	case 5:
-- 
2.17.1


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

[PATCH v1 8/9] usb: musb: fix possible out of bounds access

[Oleksij Rempel]

Either the condition 'epnum>=((u8)16)' is redundant or the array
'musb->endpoints[16]' is accessed at index 16, which is out of bounds.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 drivers/usb/musb/musb_gadget_ep0.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
index feaa85645..c8f55ac32 100644
--- a/drivers/usb/musb/musb_gadget_ep0.c
+++ b/drivers/usb/musb/musb_gadget_ep0.c
@@ -110,6 +110,11 @@ static int service_tx_status_request(
 			break;
 		}
 
+		if (epnum >= MUSB_C_NUM_EPS) {
+			handled = -EINVAL;
+			break;
+		}
+
 		is_in = epnum & USB_DIR_IN;
 		if (is_in) {
 			epnum &= 0x0f;
@@ -119,7 +124,7 @@ static int service_tx_status_request(
 		}
 		regs = musb->endpoints[epnum].regs;
 
-		if (epnum >= MUSB_C_NUM_EPS || !ep->desc) {
+		if (!ep->desc) {
 			handled = -EINVAL;
 			break;
 		}
-- 
2.17.1


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

[PATCH v1 9/9] lib: lodepng: remove useless test

[Oleksij Rempel]

Either the condition 'code_ll==(unsigned int)(-1)' is redundant or the
array 'LENGTHBASE[29]' is accessed at index 4294967038, which is out of
bounds.

On other hand this condition is tested within other scope:
"if(code_ll >= FIRST_LENGTH_CODE_INDEX && code_ll <= LAST_LENGTH_CODE_INDEX)"
with is already limited to (code_ll >= 257 && code_ll <= 285),
so it can't be -1 any way.

Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
---
 lib/gui/lodepng.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/lib/gui/lodepng.c b/lib/gui/lodepng.c
index 9cc59d709..78a34db47 100644
--- a/lib/gui/lodepng.c
+++ b/lib/gui/lodepng.c
@@ -1140,13 +1140,7 @@ static unsigned inflateHuffmanBlock(ucvector* out, const unsigned char* in, size
       code_d = huffmanDecodeSymbol(in, bp, &tree_d, inbitlength);
       if(code_d > 29)
       {
-        if(code_ll == (unsigned)(-1)) /*huffmanDecodeSymbol returns (unsigned)(-1) in case of error*/
-        {
-          /*return error code 10 or 11 depending on the situation that happened in huffmanDecodeSymbol
-          (10=no endcode, 11=wrong jump outside of tree)*/
-          error = (*bp) > inlength * 8 ? 10 : 11;
-        }
-        else error = 18; /*error: invalid distance code (30-31 are never used)*/
+        error = 18; /*error: invalid distance code (30-31 are never used)*/
         break;
       }
       distance = DISTANCEBASE[code_d];
-- 
2.17.1


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

Re: [PATCH v1 0/9] random fixes

[Sascha Hauer]

On Tue, Nov 20, 2018 at 09:07:05PM +0100, Oleksij Rempel wrote:
> next round of random fixes reported by codacy:
> https://app.codacy.com/app/olerem/barebox/issues/index?bid=9803255
> 
> Oleksij Rempel (9):
>   net: e1000: fix "Uninitialized variable: phy_data" warning
>   pinctrl: tegra30: fix "Possible null pointer dereference: group"
>     warning
>   fs: closedir: remove uninitialized variable ret
>   fs: fix possible null pointer dereference of base.
>   mfd: da9063: fix wrong NULL pointer test
>   usb: at91_udc: remove useless NULL check
>   mtd: atmel_nand: remove erroneous case
>   usb: musb: fix possible out of bounds access
>   lib: lodepng: remove useless test

Applied, thanks

Sascha

> 
>  drivers/mfd/da9063.c               | 2 +-
>  drivers/mtd/nand/atmel_nand.c      | 1 -
>  drivers/net/e1000/main.c           | 2 +-
>  drivers/pinctrl/pinctrl-tegra30.c  | 4 ++--
>  drivers/usb/gadget/at91_udc.c      | 3 +--
>  drivers/usb/musb/musb_gadget_ep0.c | 7 ++++++-
>  fs/fs.c                            | 7 +++----
>  lib/gui/lodepng.c                  | 8 +-------
>  8 files changed, 15 insertions(+), 19 deletions(-)
> 
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

[Lucas Stach]

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

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

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 3182 bytes --]

On Wed, Nov 21, 2018 at 10:16:58AM +0100, Lucas Stach wrote:
> 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.

Probably I still do not understand it:

static const struct pinctrl_tegra30_drvdata tegra124_drvdata = {
	.pingrps = tegra124_pin_groups,
	.num_pingrps = ARRAY_SIZE(tegra124_pin_groups),
	.drvgrps = tegra124_drive_groups,
	.num_drvgrps = ARRAY_SIZE(tegra124_drive_groups), 
	^^^^^ this is constant.
};

static int pinctrl_tegra30_set_drvstate(struct pinctrl_tegra30 *ctrl,
                                        struct device_node *np)
{
	const char *pins = NULL;
	const struct tegra_drive_pingroup *group = NULL;
	^^^^ here we init *group to NULL

	int hsm = -1, schmitt = -1, pds = -1, pus = -1, srr = -1, srf = -1;
	int i;
	u32 __iomem *regaddr;
	u32 val;

	if (of_property_read_string(np, "nvidia,pins", &pins))
		return 0;

	for (i = 0; i < ctrl->drvdata->num_drvgrps; i++) {
	^^^^ here we init i
		if (!strcmp(pins, ctrl->drvdata->drvgrps[i].name)) {
			group = &ctrl->drvdata->drvgrps[i];
			^^^^^ -- only here group is not NULL
			break;
		}
	}
	/* if no matching drivegroup is found */
	if (i == ctrl->drvdata->num_drvgrps)
	^^^^^ if i == num_drvgrps, group is also NULL..
		return 0;

I don't see any technical problems. Or i do oversee some thing?

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

Well, amount of real problems found by this code check is still higher.
So why not to use it?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

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

[Lucas Stach]

Am Mittwoch, den 21.11.2018, 10:40 +0100 schrieb Oleksij Rempel:
> On Wed, Nov 21, 2018 at 10:16:58AM +0100, Lucas Stach wrote:
> > 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.
> 
> Probably I still do not understand it:
> 
> static const struct pinctrl_tegra30_drvdata tegra124_drvdata = {
> 	.pingrps = tegra124_pin_groups,
> 	.num_pingrps = ARRAY_SIZE(tegra124_pin_groups),
> 	.drvgrps = tegra124_drive_groups,
> 	.num_drvgrps = ARRAY_SIZE(tegra124_drive_groups), 
> 	^^^^^ this is constant.
> };
> 
> static int pinctrl_tegra30_set_drvstate(struct pinctrl_tegra30 *ctrl,
>                                         struct device_node *np)
> {
> 	const char *pins = NULL;
> 	const struct tegra_drive_pingroup *group = NULL;
> 	^^^^ here we init *group to NULL
> 
> 	int hsm = -1, schmitt = -1, pds = -1, pus = -1, srr = -1, srf = -1;
> 	int i;
> 	u32 __iomem *regaddr;
> 	u32 val;
> 
> 	if (of_property_read_string(np, "nvidia,pins", &pins))
> > 		return 0;
> 
> 	for (i = 0; i < ctrl->drvdata->num_drvgrps; i++) {
> 	^^^^ here we init i
> > 		if (!strcmp(pins, ctrl->drvdata->drvgrps[i].name)) {
> > 			group = &ctrl->drvdata->drvgrps[i];
> > 			^^^^^ -- only here group is not NULL
> > 			break;
> > 		}
> 	}
> 	/* if no matching drivegroup is found */
> 	if (i == ctrl->drvdata->num_drvgrps)
> 	^^^^^ if i == num_drvgrps, group is also NULL..
> > 		return 0;
> 
> I don't see any technical problems. Or i do oversee some thing?

Sorry, I've looked at the wrong function. This patch doesn't break
anything, but now makes the code inconsistent. In
pinctrl_tegra30_set_state() this idiom is needed to make the code
correct and I would rather have the groups searches consistent between
the 2 functions.

> > This is a prime example where static checker warnings can prompt wrong
> > fixes. Frankly codacy should smart up to correctly analyze the
> > controlflow interdependency.
> 
> Well, amount of real problems found by this code check is still higher.
> So why not to use it?

I'm not arguing against static checkers, but I'm saying that it can
sometimes be hard to get to the real issues, when the checker is
producing a significant amount of false positives. And I'm against
changing code just to make checkers happy, because every one of them
gets something different wrong...

But whatever, the code is still correct, so the patch can stay.

Regards,
Lucas



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