Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

[Dan Carpenter]

On Mon, Dec 05, 2016 at 07:10:45PM -0500, Brian Masney wrote:
> On Mon, Dec 05, 2016 at 11:53:39PM +0300, Dan Carpenter wrote:
> > On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> > > Fixed warning found by make W=2 to reduce the amount of build noise:
> > > 
> > > warning: comparison between signed and unsigned integer expressions
> > > [-Wsign-compare]
> > 
> > Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
> > stupid.  Better to just grep -v this warning instead of trying to please
> > a broken static analysis.  Warnings like this are why it's disabled by
> > default.
> 
> Hi Dan,
>I would normally agree, however there could be a case where this
> warning flags a legitimate issue. It is obviously not an issue in this
> case. Since I'm already working on cleaning up this driver to move it
> out of staging, I figured that I would make sure that it builds cleanly
> with W=2. This was the only warning found in that driver. The
> change is harmless in my opinion and it may eliminate a nonsense warning
> for someone else down the road when doing security audits.

Iterators should be int unless there is a specific reason for a fancier
data type.  Using complicated types just makes the code more complicated
and tiring to read.

Smatch or other similar static analysis tools know that "sel" is in the
0-7 range and that ARRAY_SIZE(prox_period) is 8.  GCC almost certainly
knows this as well.  The warning messages is just printed because the
devs are lazy.  It's totally pointless.

Don't work around lazy static analysis.  It sends the wrong message to
do pointless things.

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

[Brian Masney]

On Mon, Dec 05, 2016 at 11:53:39PM +0300, Dan Carpenter wrote:
> On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> > Fixed warning found by make W=2 to reduce the amount of build noise:
> > 
> > warning: comparison between signed and unsigned integer expressions
> > [-Wsign-compare]
> 
> Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
> stupid.  Better to just grep -v this warning instead of trying to please
> a broken static analysis.  Warnings like this are why it's disabled by
> default.

Hi Dan,
   I would normally agree, however there could be a case where this
warning flags a legitimate issue. It is obviously not an issue in this
case. Since I'm already working on cleaning up this driver to move it
out of staging, I figured that I would make sure that it builds cleanly
with W=2. This was the only warning found in that driver. The
change is harmless in my opinion and it may eliminate a nonsense warning
for someone else down the road when doing security audits.

   This driver doesn't need much to move it out of staging. Most of the
patches in this series were trivial cleanups and not interesting at all.
Since I already have one of these devices, I figured that I'd do the
grunt work to get it out of staging. My goal with the upcoming final
patch that moves it out of staging is to reduce the amount of code churn
in the driver once it graduates from staging.

Brian

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

[Dan Carpenter]

On Sat, Dec 03, 2016 at 09:19:36PM -0500, Brian Masney wrote:
> Fixed warning found by make W=2 to reduce the amount of build noise:
> 
> warning: comparison between signed and unsigned integer expressions
> [-Wsign-compare]

Ugh...  Please don't do work arounds for nonsense warnings.  W=2 is so
stupid.  Better to just grep -v this warning instead of trying to please
a broken static analysis.  Warnings like this are why it's disabled by
default.

regards,
dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers

[Brian Masney]

Fixed warning found by make W=2 to reduce the amount of build noise:

warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]

Signed-off-by: Brian Masney 
---
 drivers/staging/iio/light/isl29028.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/isl29028.c 
b/drivers/staging/iio/light/isl29028.c
index 218d165..23a4db8 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -80,7 +80,7 @@ static int isl29028_set_proxim_sampling(struct isl29028_chip 
*chip,
unsigned int sampling)
 {
static unsigned int prox_period[] = {800, 400, 200, 100, 75, 50, 12, 0};
-   int sel;
+   unsigned int sel;
unsigned int period = DIV_ROUND_UP(1000, sampling);
 
for (sel = 0; sel < ARRAY_SIZE(prox_period); ++sel) {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel