2012-05-21 15:44:18

by Reinhard Tartler

[permalink] [raw]
Subject: [PATCH] drivers/leds/leds-lp5521.c: actually check return value of lp5521_read

This detects device failures properly. Fixes compiler warning:
drivers/leds/leds-lp5521.c:741: warning: 'buf' may be used uninitialized in this function

Signed-off-by: Reinhard Tartler <[email protected]>
---
drivers/leds/leds-lp5521.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

This problem was found by tools developed by the VAMOS project:
http://www4.cs.fau.de/Research/VAMOS/

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 410a723..943c69d 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -785,7 +785,7 @@ static int __devinit lp5521_probe(struct i2c_client *client,
* LP5521_REG_ENABLE register will not have any effect - strange!
*/
ret = lp5521_read(client, LP5521_REG_R_CURRENT, &buf);
- if (buf != LP5521_REG_R_CURR_DEFAULT) {
+ if (ret == -EIO || buf != LP5521_REG_R_CURR_DEFAULT) {
dev_err(&client->dev, "error in resetting chip\n");
goto fail2;
}
--
1.7.10


2012-05-22 03:28:11

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] drivers/leds/leds-lp5521.c: actually check return value of lp5521_read

On Mon, May 21, 2012 at 11:35 PM, Reinhard Tartler <[email protected]> wrote:
> This detects device failures properly. Fixes compiler warning:
> drivers/leds/leds-lp5521.c:741: warning: 'buf' may be used uninitialized in this function
>
> Signed-off-by: Reinhard Tartler <[email protected]>
> ---
> ?drivers/leds/leds-lp5521.c | ? ?2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> This problem was found by tools developed by the VAMOS project:
> http://www4.cs.fau.de/Research/VAMOS/
>
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index 410a723..943c69d 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -785,7 +785,7 @@ static int __devinit lp5521_probe(struct i2c_client *client,
> ? ? ? ? * LP5521_REG_ENABLE register will not have any effect - strange!
> ? ? ? ? */
> ? ? ? ?ret = lp5521_read(client, LP5521_REG_R_CURRENT, &buf);
> - ? ? ? if (buf != LP5521_REG_R_CURR_DEFAULT) {
> + ? ? ? if (ret == -EIO || buf != LP5521_REG_R_CURR_DEFAULT) {

Looks fine to me.
Acked-by: Bryan Wu <[email protected]>

And applied to fixes-for-3.5 branch of linux-leds.git

-Bryan

2012-05-30 09:25:27

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] drivers/leds/leds-lp5521.c: actually check return value of lp5521_read

Sorry, Reinhard, I just found this issue was fixed by Dan Carpenter in
mainline now, (commit 5bc9ad774c063f6b41965e7314f2c26aa5e465a0)

So I have to drop this.

-Bryan

On Tue, May 22, 2012 at 11:27 AM, Bryan Wu <[email protected]> wrote:
> On Mon, May 21, 2012 at 11:35 PM, Reinhard Tartler <[email protected]> wrote:
>> This detects device failures properly. Fixes compiler warning:
>> drivers/leds/leds-lp5521.c:741: warning: 'buf' may be used uninitialized in this function
>>
>> Signed-off-by: Reinhard Tartler <[email protected]>
>> ---
>> ?drivers/leds/leds-lp5521.c | ? ?2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> This problem was found by tools developed by the VAMOS project:
>> http://www4.cs.fau.de/Research/VAMOS/
>>
>> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
>> index 410a723..943c69d 100644
>> --- a/drivers/leds/leds-lp5521.c
>> +++ b/drivers/leds/leds-lp5521.c
>> @@ -785,7 +785,7 @@ static int __devinit lp5521_probe(struct i2c_client *client,
>> ? ? ? ? * LP5521_REG_ENABLE register will not have any effect - strange!
>> ? ? ? ? */
>> ? ? ? ?ret = lp5521_read(client, LP5521_REG_R_CURRENT, &buf);
>> - ? ? ? if (buf != LP5521_REG_R_CURR_DEFAULT) {
>> + ? ? ? if (ret == -EIO || buf != LP5521_REG_R_CURR_DEFAULT) {
>
> Looks fine to me.
> Acked-by: Bryan Wu <[email protected]>
>
> And applied to fixes-for-3.5 branch of linux-leds.git
>
> -Bryan



--
Bryan Wu <[email protected]>
Kernel Developer ? ?+86.186-168-78255 Mobile
Canonical Ltd. ? ? ?http://www.canonical.com
Ubuntu - Linux for human beings | http://www.ubuntu.com