Skip to contentSkip to conversation

misc/bh1770glc: error handling in bh1770_power_state_store()

PATCH In review Dan Carpenter 2 participants 2 messages Oct 28, 2010 – Oct 29, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 misc/bh1770glc: error handling in bh1770_power_state_store()

There was a signedness bug so "ret" was never less than zero and that breaks the error handling. Also in the original code it would overwrite ret and the result is still negative but it's bogus number instead of the correct error code.

drivers/misc/bh1770glc.c +6 -2
index cee632e..d79a972 100644
@@ -649,7 +649,7 @@ static ssize_t bh1770_power_state_store(struct device *dev,
 {
 	struct bh1770_chip *chip =  dev_get_drvdata(dev);
 	unsigned long value;
-	size_t ret;
+	ssize_t ret;
 
 	if (strict_strtoul(buf, 0, &value))
 		return -EINVAL;
@@ -659,8 +659,12 @@ static ssize_t bh1770_power_state_store(struct device *dev,
 		pm_runtime_get_sync(dev);
 
 		ret = bh1770_lux_rate(chip, chip->lux_rate_index);
-		ret |= bh1770_lux_interrupt_control(chip, BH1770_ENABLE);
+		if (ret < 0) {
+			pm_runtime_put(dev);
+			goto leave;
+		}
 
+		ret = bh1770_lux_interrupt_control(chip, BH1770_ENABLE);
 		if (ret < 0) {
 			pm_runtime_put(dev);
 			goto leave;
Acked-by on v1 1/1
#1 lore raw
Quoted 6 lines · ext Dan Carpenter

There was a signedness bug so "ret" was never less than zero and that breaks the error handling. Also in the original code it would overwrite ret and the result is still negative but it's bogus number instead of the correct error code.

Quoted 28 lines
drivers/misc/bh1770glc.c +6 -2
index cee632e..d79a972 100644
@@ -649,7 +649,7 @@ static ssize_t bh1770_power_state_store(struct device *dev,
 {
 	struct bh1770_chip *chip =  dev_get_drvdata(dev);
 	unsigned long value;
-	size_t ret;
+	ssize_t ret;
 
 	if (strict_strtoul(buf, 0, &value))
 		return -EINVAL;
@@ -659,8 +659,12 @@ static ssize_t bh1770_power_state_store(struct device *dev,
 		pm_runtime_get_sync(dev);
 
 		ret = bh1770_lux_rate(chip, chip->lux_rate_index);
-		ret |= bh1770_lux_interrupt_control(chip, BH1770_ENABLE);
+		if (ret < 0) {
+			pm_runtime_put(dev);
+			goto leave;
+		}
 
+		ret = bh1770_lux_interrupt_control(chip, BH1770_ENABLE);
 		if (ret < 0) {
 			pm_runtime_put(dev);
 			goto leave;