Re: [PATCH] hwmon: applesmc: check status earlier.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tom,

From: Tom Rix <trix@xxxxxxxxxx>

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@xxxxxxxxxx>
---
  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@xxxxxxxxxxx>

Henrik



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux