Message ID | 20210422202857.15798-1-dariobin@libero.it |
---|---|
State | Accepted |
Commit | 48594c38edb235d671c544faf922b29a1c1a5d23 |
Delegated to: | Tom Rini |
Headers | show |
Series | [v2] pinctrl: single: fix a never true comparison | expand |
On 22/04/21 10:28PM, Dario Binacchi wrote: > As reported by Coverity Scan for Das U-Boot, the 'less-than-zero' > comparison of an unsigned value is never true. > > Signed-off-by: Dario Binacchi <dariobin@libero.it> > Reviewed-by: Pratyush Yadav <p.yadav@ti.com> > > --- > > Changes in v2: > - Balance quote in commit message > - Add review tag > > drivers/pinctrl/pinctrl-single.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c > index 48bdd0f6f5..7e1c5bb0a5 100644 > --- a/drivers/pinctrl/pinctrl-single.c > +++ b/drivers/pinctrl/pinctrl-single.c > @@ -295,7 +295,7 @@ static int single_configure_pins(struct udevice *dev, > func->npins = 0; > for (n = 0; n < count; n++, pins++) { > offset = fdt32_to_cpu(pins->reg); > - if (offset < 0 || offset > pdata->offset) { > + if (offset > pdata->offset) { I went back and took a look at it again. The "offset < 0" comes from "reg < 0" in 9b884e79a6 (pinctrl: single: fix offset management, 2021-04-11), where reg is of type phys_addr_t. But phys_addr_t is also an unsigned value so I suppose the check was wrong to begin with. And fdt32_to_cpu() just does byte order conversions so it shouldn't return a negative value because there is no scope for failure. So again, the patch looks good to me, but some added context for anyone interested. > dev_err(dev, " invalid register offset 0x%x\n", > offset); > continue; > @@ -344,7 +344,7 @@ static int single_configure_bits(struct udevice *dev, > func->npins = 0; > for (n = 0; n < count; n++, pins++) { > offset = fdt32_to_cpu(pins->reg); > - if (offset < 0 || offset > pdata->offset) { > + if (offset > pdata->offset) { > dev_dbg(dev, " invalid register offset 0x%x\n", > offset); > continue; > -- > 2.17.1 >
On Thu, Apr 22, 2021 at 10:28:56PM +0200, Dario Binacchi wrote: > As reported by Coverity Scan for Das U-Boot, the 'less-than-zero' > comparison of an unsigned value is never true. > > Signed-off-by: Dario Binacchi <dariobin@libero.it> > Reviewed-by: Pratyush Yadav <p.yadav@ti.com> Applied to u-boot/master, thanks!
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 48bdd0f6f5..7e1c5bb0a5 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -295,7 +295,7 @@ static int single_configure_pins(struct udevice *dev, func->npins = 0; for (n = 0; n < count; n++, pins++) { offset = fdt32_to_cpu(pins->reg); - if (offset < 0 || offset > pdata->offset) { + if (offset > pdata->offset) { dev_err(dev, " invalid register offset 0x%x\n", offset); continue; @@ -344,7 +344,7 @@ static int single_configure_bits(struct udevice *dev, func->npins = 0; for (n = 0; n < count; n++, pins++) { offset = fdt32_to_cpu(pins->reg); - if (offset < 0 || offset > pdata->offset) { + if (offset > pdata->offset) { dev_dbg(dev, " invalid register offset 0x%x\n", offset); continue;