On Thu, Aug 20, 2020 at 06:19:32AM -0700, [email protected] wrote:
> From: Tom Rix <[email protected]>
>
> 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 <[email protected]>
Applied.
Thanks,
Guenter
> ---
> 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,
