* [patch] iio:dac:ad5449: unlock on error path
@ 2012-10-24 7:13 Dan Carpenter
2012-10-24 8:22 ` Lars-Peter Clausen
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2012-10-24 7:13 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Lars-Peter Clausen, linux-iio, kernel-janitors
There is an unlock missing on this error path.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Only needed in linux-next.
diff --git a/drivers/iio/dac/ad5449.c b/drivers/iio/dac/ad5449.c
index 5b43030..0ee6f8e 100644
--- a/drivers/iio/dac/ad5449.c
+++ b/drivers/iio/dac/ad5449.c
@@ -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,
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] iio:dac:ad5449: unlock on error path
2012-10-24 7:13 [patch] iio:dac:ad5449: unlock on error path Dan Carpenter
@ 2012-10-24 8:22 ` Lars-Peter Clausen
2012-11-02 11:06 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2012-10-24 8:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jonathan Cameron, linux-iio, kernel-janitors
On 10/24/2012 09:13 AM, Dan Carpenter wrote:
> There is an unlock missing on this error path.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks,
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> Only needed in linux-next.
>
> diff --git a/drivers/iio/dac/ad5449.c b/drivers/iio/dac/ad5449.c
> index 5b43030..0ee6f8e 100644
> --- a/drivers/iio/dac/ad5449.c
> +++ b/drivers/iio/dac/ad5449.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] iio:dac:ad5449: unlock on error path
2012-10-24 8:22 ` Lars-Peter Clausen
@ 2012-11-02 11:06 ` Jonathan Cameron
2012-11-02 16:52 ` walter harms
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2012-11-02 11:06 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Dan Carpenter, Jonathan Cameron, linux-iio, kernel-janitors
On 10/24/2012 09:22 AM, Lars-Peter Clausen wrote:
> On 10/24/2012 09:13 AM, Dan Carpenter wrote:
>> There is an unlock missing on this error path.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Thanks,
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Thanks applied to togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
>
>> ---
>> Only needed in linux-next.
>>
>> diff --git a/drivers/iio/dac/ad5449.c b/drivers/iio/dac/ad5449.c
>> index 5b43030..0ee6f8e 100644
>> --- a/drivers/iio/dac/ad5449.c
>> +++ b/drivers/iio/dac/ad5449.c
>> @@ -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 majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] iio:dac:ad5449: unlock on error path
2012-11-02 11:06 ` Jonathan Cameron
@ 2012-11-02 16:52 ` walter harms
2012-11-05 6:38 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: walter harms @ 2012-11-02 16:52 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Dan Carpenter, Jonathan Cameron, linux-iio,
kernel-janitors
Am 02.11.2012 12:06, schrieb Jonathan Cameron:
> On 10/24/2012 09:22 AM, Lars-Peter Clausen wrote:
>> On 10/24/2012 09:13 AM, Dan Carpenter wrote:
>>> There is an unlock missing on this error path.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> Thanks,
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> Thanks applied to togreg branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
>
>>
>>> ---
>>> Only needed in linux-next.
>>>
>>> diff --git a/drivers/iio/dac/ad5449.c b/drivers/iio/dac/ad5449.c
>>> index 5b43030..0ee6f8e 100644
>>> --- a/drivers/iio/dac/ad5449.c
>>> +++ b/drivers/iio/dac/ad5449.c
>>> @@ -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,
>>> --
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] iio:dac:ad5449: unlock on error path
2012-11-02 16:52 ` walter harms
@ 2012-11-05 6:38 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-11-05 6:38 UTC (permalink / raw)
To: walter harms
Cc: Jonathan Cameron, Lars-Peter Clausen, Jonathan Cameron, linux-iio,
kernel-janitors
On Fri, Nov 02, 2012 at 05:52:26PM +0100, walter harms wrote:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-05 6:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-24 7:13 [patch] iio:dac:ad5449: unlock on error path Dan Carpenter
2012-10-24 8:22 ` Lars-Peter Clausen
2012-11-02 11:06 ` Jonathan Cameron
2012-11-02 16:52 ` walter harms
2012-11-05 6:38 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).