lkml.org 
[lkml]   [2017]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] iio staging: tsl2x7x: clean up limit checks
On Mon, 21 Aug 2017 13:11:03 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> The second part of this patch is probably the most interesting. We
> use "TSL2X7X_MAX_LUX_TABLE_SIZE * 3" as the limit instead of just
> "TSL2X7X_MAX_LUX_TABLE_SIZE". It creates a static checker warning that
> we are going of of bounds, but in real life we always hit the break
> statement on the last element so it's fine.
>
> The situation is that we normally have arrays with 3 elements of struct
> tsl2x7x_lux which has 3 unsigned integers. If we load the table with
> sysfs then we're allow to have 9 elements instead.
>
> So the size of the default table in bytes is sizeof(int) times 3 struct
> members times 3 elements. The original code wrote it as sizeof(int)
> times the number of elements in the bigger table (9). It happens that
> 9 is the same thing as 3 * 3 but expressing it that way is misleading.
>
> For the second part of the patch, the original code just had an extra
> "multiply by three" and now that is removed. The last element in the
> array is always zeroed memory whether this uses the default tables or it
> gets loaded with sysfs so we always hit the break statement anyway.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Looks sensible to me.

Cc'd Brian who has been working extensively on this driver recently as I'd
like his input.

Jonathan
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index ecae92211216..1beb8d2eb848 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,11 @@ struct tsl2x7x_lux {
> unsigned int ch1;
> };
>
> +/* Max number of segments allowable in LUX table */
> +#define TSL2X7X_MAX_LUX_TABLE_SIZE 9
> +/* The default tables are all 3 elements */
> +#define MAX_DEFAULT_TABLE_BYTES (sizeof(struct tsl2x7x_lux) * 3)
> +
> /**
> * 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..2db1715ff659 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -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,

\
 
 \ /
  Last update: 2017-09-03 13:36    [W:0.049 / U:1.248 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site