iio:dac:ad5449: unlock on error path

PATCH Applied Dan Carpenter 4 participants 5 messages Oct 24, 2012 – Nov 5, 2012

Subsystems (3)

Kernel janitors IIO Analog Devices IIO drivers

lore

Share & tools

lore

Dan Carpenter

author

2012-10-24 07:13 UTC #0 lore raw

PATCH 1/1 iio:dac:ad5449: unlock on error path

There is an unlock missing on this error path.

Signed-off-by Dan Carpenter <[email protected]>

---
Only needed in linux-next.

drivers/iio/dac/ad5449.c +4 -3

index 5b43030..0ee6f8e 100644
@@ -124,12 +124,13 @@ static int ad5449_read(struct iio_dev *indio_dev, unsigned int addr,
 
 	ret = spi_sync(st->spi, &msg);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	*val = be16_to_cpu(st->data[1]);
-	mutex_unlock(&indio_dev->mlock);
 
-	return 0;
+out_unlock:
+	mutex_unlock(&indio_dev->mlock);
+	return ret;
 }
 
 static int ad5449_read_raw(struct iio_dev *indio_dev,

Lars-Peter Clausen

Acked-by on v1 1/1

2012-10-24 08:22 UTC #1 lore raw

Quoted 3 lines · Dan Carpenter

There is an unlock missing on this error path.
Signed-off-by Dan Carpenter <[email protected]>

Thanks,

Acked-by Lars-Peter Clausen <[email protected]>

Quoted 28 lines

---
Only needed in linux-next.
drivers/iio/dac/ad5449.c +4 -4
index 5b43030..0ee6f8e 100644
@@ -124,12 +124,13 @@ static int ad5449_read(struct iio_dev *indio_dev, unsigned int addr,
 
 	ret = spi_sync(st->spi, &msg);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	*val = be16_to_cpu(st->data[1]);
-	mutex_unlock(&indio_dev->mlock);
 
-	return 0;
+out_unlock:
+	mutex_unlock(&indio_dev->mlock);
+	return ret;
 }
 
 static int ad5449_read_raw(struct iio_dev *indio_dev,
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jonathan Cameron

maintainer Applied

2012-11-02 11:06 UTC #2 lore raw

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Dan Carpenter
There is an unlock missing on this error path.
Signed-off-by Dan Carpenter <[email protected]>
Thanks,
Acked-by Lars-Peter Clausen <[email protected]>

Thanks applied to togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Quoted 35 lines

Quoted 26 lines
---
Only needed in linux-next.
drivers/iio/dac/ad5449.c +4 -4
index 5b43030..0ee6f8e 100644
@@ -124,12 +124,13 @@ static int ad5449_read(struct iio_dev *indio_dev, unsigned int addr,
 
 	ret = spi_sync(st->spi, &msg);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	*val = be16_to_cpu(st->data[1]);
-	mutex_unlock(&indio_dev->mlock);
 
-	return 0;
+out_unlock:
+	mutex_unlock(&indio_dev->mlock);
+	return ret;
 }
 
 static int ad5449_read_raw(struct iio_dev *indio_dev,
--
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

walter harms

2012-11-02 16:52 UTC #3 lore raw

Am 02.11.2012 12:06, schrieb Jonathan Cameron:

Quoted 30 lines · Lars-Peter Clausen

Quoted 8 lines · Dan Carpenter
Quoted 3 lines
There is an unlock missing on this error path.
Signed-off-by Dan Carpenter <[email protected]>
Thanks,
Acked-by Lars-Peter Clausen <[email protected]>
Thanks applied to togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
Quoted 18 lines
Quoted 18 lines
---
Only needed in linux-next.
drivers/iio/dac/ad5449.c +1 -2
index 5b43030..0ee6f8e 100644
@@ -124,12 +124,13 @@ static int ad5449_read(struct iio_dev *indio_dev, unsigned int addr,
 
 	ret = spi_sync(st->spi, &msg);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	*val = be16_to_cpu(st->data[1]);
-	mutex_unlock(&indio_dev->mlock);
 

Quoted 8 lines

Quoted 8 lines
drivers/iio/dac/ad5449.c static int ad5449_read(struct iio_dev *indio_dev, unsigned int addr,
Quoted 8 lines
drivers/iio/dac/ad5449.c +3 -2
-	return 0;
+out_unlock:
+	mutex_unlock(&indio_dev->mlock);
+	return ret;
 }
 
 static int ad5449_read_raw(struct iio_dev *indio_dev,
--

if i understand the code correct you can simply the flow by turning the check on its head.

ret = spi_sync(st->spi, &msg);
if (ret>=0)

    *val = be16_to_cpu(st->data[1]);

mutex_unlock(&indio_dev->mlock);
return ret;

re,
wh

Dan Carpenter

author

2012-11-05 06:38 UTC #4 lore raw

Quoted 8 lines · walter harms

if i understand the code correct you can simply the flow by turning the check on its head.
ret = spi_sync(st->spi, &msg);
if (ret>=0)
    *val = be16_to_cpu(st->data[1]);
mutex_unlock(&indio_dev->mlock);
return ret;

It's better to always treat errors as the special case and success as the normal case. It makes the code more readable if every check is consistent.

regards,
dan carpenter