public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Jonathan Cameron <jic23@kernel.org>,
	Brian Masney <masneyb@onstation.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	simran singhal <singhalsimran0@gmail.com>,
	Eva Rachel Retuya <eraretuya@gmail.com>,
	Enric Balletbo i Serra <eballetbo@gmail.com>,
	Paolo Cretaro <melko@frugalware.org>,
	linux-iio@vger.kernel.org, devel@driverdev.osuosl.org,
	kernel-janitors@vger.kernel.org
Subject: [PATCH v2] staging: iio: tsl2x7x: clean up limit checks
Date: Fri, 08 Sep 2017 10:53:43 +0000	[thread overview]
Message-ID: <20170908105343.tk3j36sgxd7xm222@mwanda> (raw)
In-Reply-To: <20170905233123.vfrb4j4bnhtgcrrt@fd32499230b2>

The background of this code is that we can either use the default
tables or load our own table with sysfs.  The default tables are three
element arrays of struct tsl2x7x_lux.  If we load the table with sysfs
then we can have as many as nine elements.  Which ever way we do it, the
last element is always zeroed out.

The most interesting part of this patch is in the
in_illuminance0_lux_table_show() function.  We were using the wrong
limit, "TSL2X7X_MAX_LUX_TABLE_SIZE * 3", when it should have been just
"TSL2X7X_MAX_LUX_TABLE_SIZE".  This creates a static checker warning
that we are going of of bounds.  However, since the last element is
always zeroed out, that means we hit the break statement and the code
works correctly despite the wrong limit check.

I made several related readability changes.  The most notable that I
changed the MAX_DEFAULT_TABLE_BYTES define which was:

#define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)

I renamed the define to TSL2X7X_DEFAULT_TABLE_BYTES because it's not the
max size, it's the only size.  Also the size should really be expressed
as sizeof(struct tsl2x7x_lux) * 3.  In other words, 12 * 3 instead of
4 * 9.  It's 36 bytes either way, so this doesn't change the behavior.

Finally, I created the TSL2X7X_DEF_LUX_TABLE_SZ define instead of using
the magic number 3.  I declared the default tables using that define
to hopefully signal to future programmers that if they want to use a
different size they have to update all the related code.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
index ecae92211216..a216c6943a84 100644
--- a/drivers/staging/iio/light/tsl2x7x.h
+++ b/drivers/staging/iio/light/tsl2x7x.h
@@ -23,10 +23,6 @@
 #define __TSL2X7X_H
 #include <linux/pm.h>
 
-/* Max number of segments allowable in LUX table */
-#define TSL2X7X_MAX_LUX_TABLE_SIZE		9
-#define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)
-
 struct iio_dev;
 
 struct tsl2x7x_lux {
@@ -35,6 +31,13 @@ struct tsl2x7x_lux {
 	unsigned int ch1;
 };
 
+/* Max number of segments allowable in LUX table */
+#define TSL2X7X_MAX_LUX_TABLE_SIZE		9
+/* The default LUX tables all have 3 elements.  */
+#define TSL2X7X_DEF_LUX_TABLE_SZ		3
+#define TSL2X7X_DEFAULT_TABLE_BYTES (sizeof(struct tsl2x7x_lux) * \
+				     TSL2X7X_DEF_LUX_TABLE_SZ)
+
 /**
  * struct tsl2x7x_default_settings - power on defaults unless
  *                                   overridden by platform data.
diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 786e93f16ce9..b3a9efecf536 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -192,25 +192,25 @@ struct tsl2X7X_chip {
 };
 
 /* Different devices require different coefficents */
-static const struct tsl2x7x_lux tsl2x71_lux_table[] = {
+static const struct tsl2x7x_lux tsl2x71_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ] = {
 	{ 14461,   611,   1211 },
 	{ 18540,   352,    623 },
 	{     0,     0,      0 },
 };
 
-static const struct tsl2x7x_lux tmd2x71_lux_table[] = {
+static const struct tsl2x7x_lux tmd2x71_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ] = {
 	{ 11635,   115,    256 },
 	{ 15536,    87,    179 },
 	{     0,     0,      0 },
 };
 
-static const struct tsl2x7x_lux tsl2x72_lux_table[] = {
+static const struct tsl2x7x_lux tsl2x72_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ] = {
 	{ 14013,   466,   917 },
 	{ 18222,   310,   552 },
 	{     0,     0,     0 },
 };
 
-static const struct tsl2x7x_lux tmd2x72_lux_table[] = {
+static const struct tsl2x7x_lux tmd2x72_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ] = {
 	{ 13218,   130,   262 },
 	{ 17592,   92,    169 },
 	{     0,     0,     0 },
@@ -530,7 +530,7 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
 	else
 		memcpy(chip->tsl2x7x_device_lux,
 		(struct tsl2x7x_lux *)tsl2x7x_default_lux_table_group[chip->id],
-				MAX_DEFAULT_TABLE_BYTES);
+				TSL2X7X_DEFAULT_TABLE_BYTES);
 }
 
 /**
@@ -1113,7 +1113,7 @@ static ssize_t in_illuminance0_lux_table_show(struct device *dev,
 	int i = 0;
 	int offset = 0;
 
-	while (i < (TSL2X7X_MAX_LUX_TABLE_SIZE * 3)) {
+	while (i < TSL2X7X_MAX_LUX_TABLE_SIZE) {
 		offset += snprintf(buf + offset, PAGE_SIZE, "%u,%u,%u,",
 			chip->tsl2x7x_device_lux[i].ratio,
 			chip->tsl2x7x_device_lux[i].ch0,

  parent reply	other threads:[~2017-09-08 10:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-21 10:11 [PATCH] iio staging: tsl2x7x: clean up limit checks Dan Carpenter
2017-09-03 11:35 ` Jonathan Cameron
2017-09-04  2:12   ` Brian Masney
2017-09-05 14:58     ` Dan Carpenter
2017-09-05 21:02       ` Brian Masney
2017-09-05 23:31         ` Brian Masney
2017-09-06 12:41           ` Dan Carpenter
2017-09-08 10:53           ` Dan Carpenter [this message]
2017-09-08 13:48             ` [PATCH v2] staging: iio: " walter harms
2017-09-08 14:05               ` Dan Carpenter
2017-09-15 23:30             ` Brian Masney
2017-09-16 11:11             ` Paolo Cretaro
2017-09-16 11:37               ` Dan Carpenter
2017-09-16 12:18                 ` Paolo Cretaro
2017-09-16 22:05                   ` Jonathan Cameron
2017-09-18  9:58                   ` Dan Carpenter

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=20170908105343.tk3j36sgxd7xm222@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=eballetbo@gmail.com \
    --cc=eraretuya@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=masneyb@onstation.org \
    --cc=melko@frugalware.org \
    --cc=pmeerw@pmeerw.net \
    --cc=singhalsimran0@gmail.com \
    /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