lkml.org 
[lkml]   [2020]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] hwmon: applesmc: check status earlier.
From
Date
Hi Tom,

> From: Tom Rix <trix@redhat.com>
>
> clang static analysis reports this representative problem
>
> applesmc.c:758:10: warning: 1st function call argument is an
> uninitialized value
> left = be16_to_cpu(*(__be16 *)(buffer + 6)) >> 2;
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> buffer is filled by the earlier call
>
> ret = applesmc_read_key(LIGHT_SENSOR_LEFT_KEY, ...
>
> This problem is reported because a goto skips the status check.
> Other similar problems use data from applesmc_read_key before checking
> the status. So move the checks to before the use.
>
> Signed-off-by: Tom Rix <trix@redhat.com>
> ---
> drivers/hwmon/applesmc.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 316618409315..a18887990f4a 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -753,15 +753,18 @@ static ssize_t applesmc_light_show(struct device *dev,
> }
>
> ret = applesmc_read_key(LIGHT_SENSOR_LEFT_KEY, buffer, data_length);
> + if (ret)
> + goto out;
> /* newer macbooks report a single 10-bit bigendian value */
> if (data_length == 10) {
> left = be16_to_cpu(*(__be16 *)(buffer + 6)) >> 2;
> goto out;
> }
> left = buffer[2];
> +
> + ret = applesmc_read_key(LIGHT_SENSOR_RIGHT_KEY, buffer, data_length);
> if (ret)
> goto out;
> - ret = applesmc_read_key(LIGHT_SENSOR_RIGHT_KEY, buffer, data_length);
> right = buffer[2];
>
> out:
> @@ -810,12 +813,11 @@ static ssize_t applesmc_show_fan_speed(struct device *dev,
> to_index(attr));
>
> ret = applesmc_read_key(newkey, buffer, 2);
> - speed = ((buffer[0] << 8 | buffer[1]) >> 2);
> -
> if (ret)
> return ret;
> - else
> - return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", speed);
> +
> + speed = ((buffer[0] << 8 | buffer[1]) >> 2);
> + return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", speed);
> }
>
> static ssize_t applesmc_store_fan_speed(struct device *dev,
> @@ -851,12 +853,11 @@ static ssize_t applesmc_show_fan_manual(struct device *dev,
> u8 buffer[2];
>
> ret = applesmc_read_key(FANS_MANUAL, buffer, 2);
> - manual = ((buffer[0] << 8 | buffer[1]) >> to_index(attr)) & 0x01;
> -
> if (ret)
> return ret;
> - else
> - return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", manual);
> +
> + manual = ((buffer[0] << 8 | buffer[1]) >> to_index(attr)) & 0x01;
> + return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", manual);
> }
>
> static ssize_t applesmc_store_fan_manual(struct device *dev,
> @@ -872,10 +873,11 @@ static ssize_t applesmc_store_fan_manual(struct device *dev,
> return -EINVAL;
>
> ret = applesmc_read_key(FANS_MANUAL, buffer, 2);
> - val = (buffer[0] << 8 | buffer[1]);
> if (ret)
> goto out;
>
> + val = (buffer[0] << 8 | buffer[1]);
> +
> if (input)
> val = val | (0x01 << to_index(attr));
> else
> @@ -951,13 +953,12 @@ static ssize_t applesmc_key_count_show(struct device *dev,
> u32 count;
>
> ret = applesmc_read_key(KEY_COUNT_KEY, buffer, 4);
> - count = ((u32)buffer[0]<<24) + ((u32)buffer[1]<<16) +
> - ((u32)buffer[2]<<8) + buffer[3];
> -
> if (ret)
> return ret;
> - else
> - return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", count);
> +
> + count = ((u32)buffer[0]<<24) + ((u32)buffer[1]<<16) +
> + ((u32)buffer[2]<<8) + buffer[3];
> + return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", count);
> }
>
> static ssize_t applesmc_key_at_index_read_show(struct device *dev,
>

Looks good, thank you.

Reviewed-by: Henrik Rydberg <rydberg@bitmath.org>

Henrik

\
 
 \ /
  Last update: 2022-09-17 16:08    [W:0.052 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site