From: Dan Carpenter <dan.carpenter@oracle.com>
To: Brian Masney <masneyb@onstation.org>
Cc: jic23@kernel.org, linux-iio@vger.kernel.org,
devel@driverdev.osuosl.org, lars@metafoo.de,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
ldewangan@nvidia.com, pmeerw@pmeerw.net, knaack.h@gmx.de
Subject: Re: [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers
Date: Tue, 6 Dec 2016 13:19:45 +0300 [thread overview]
Message-ID: <20161206101945.GE8244@mwanda> (raw)
In-Reply-To: <20161206001045.GA29358@basecamp.onstation.org>
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
next prev parent reply other threads:[~2016-12-06 10:20 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-04 2:19 [PATCH 00/19] staging: iio: isl29028: staging cleanups Brian Masney
2016-12-04 2:19 ` [PATCH 01/19] staging: iio: isl29028: remove nested if statements Brian Masney
2016-12-04 11:01 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 02/19] staging: iio: isl29028: remove enable flag from isl29028_enable_proximity() Brian Masney
2016-12-04 11:13 ` Jonathan Cameron
2016-12-04 11:16 ` Jonathan Cameron
2017-01-14 20:00 ` Brian Masney
2017-01-15 14:33 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 03/19] staging: iio: isl29028: remove chip test and defaults from isl29028_chip_init() Brian Masney
2016-12-04 11:23 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 04/19] staging: iio: isl29028: add power management support Brian Masney
2016-12-04 11:37 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 05/19] staging: iio: isl29028: made alignment of #defines consistent Brian Masney
2016-12-04 11:39 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 06/19] staging: iio: isl29028: made alignment of variables in struct isl29028_chip consistent Brian Masney
2016-12-04 2:19 ` [PATCH 07/19] staging: iio: isl29028: fix alignment of function arguments Brian Masney
2016-12-04 2:19 ` [PATCH 08/19] staging: iio: isl29028: combine isl29028_proxim_get() and isl29028_read_proxim() Brian Masney
2016-12-04 2:19 ` [PATCH 09/19] staging: iio: isl29028: change newlines to improve readability Brian Masney
2016-12-04 2:19 ` [PATCH 10/19] staging: iio: isl29028: remove unused define ISL29028_DEV_ATTR Brian Masney
2016-12-04 2:19 ` [PATCH 11/19] staging: iio: isl29028: made column alignment in isl29028_channels consistent Brian Masney
2016-12-04 11:48 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 12/19] staging: iio: isl29028: fix comparison between signed and unsigned integers Brian Masney
2016-12-05 20:53 ` Dan Carpenter
2016-12-06 0:10 ` Brian Masney
2016-12-06 10:19 ` Dan Carpenter [this message]
2016-12-04 2:19 ` [PATCH 13/19] staging: iio: isl29028: move failure logging into isl29028_set_proxim_sampling() Brian Masney
2016-12-04 11:50 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 14/19] staging: iio: isl29028: move failure logging into isl29028_set_als_scale() Brian Masney
2016-12-04 11:51 ` Jonathan Cameron
2016-12-04 2:19 ` [PATCH 15/19] staging: iio: isl29028: made error messages consistent Brian Masney
2016-12-04 2:19 ` [PATCH 16/19] staging: iio: isl29028: remove unnecessary error logging in isl29028_chip_init_and_power_on() Brian Masney
2016-12-04 2:19 ` [PATCH 17/19] staging: iio: isl29028: remove out of memory log message Brian Masney
2016-12-04 2:19 ` [PATCH 18/19] staging: iio: isl29028: remove unnecessary parenthesis Brian Masney
2016-12-04 2:19 ` [PATCH 19/19] staging: iio: isl29028: remove legacy device tree binding Brian Masney
2016-12-04 11:55 ` Jonathan Cameron
2017-01-08 9:50 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161206101945.GE8244@mwanda \
--to=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=ldewangan@nvidia.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masneyb@onstation.org \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).