[PATCH] Fix for unintialized variables

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] Fix for unintialized variables
@ 2011-04-16  8:37 Raghavendra D Prabhu
  2011-04-16  8:43 ` [PATCH 1/2] FIX: engine_state is uninitialized Raghavendra D Prabhu
  2011-04-16  8:44 ` [PATCH 2/2] FIX: Check the return value before proceeding Raghavendra D Prabhu
  0 siblings, 2 replies; 4+ messages in thread
From: Raghavendra D Prabhu @ 2011-04-16  8:37 UTC (permalink / raw)
  To: Richard Purdie; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

Hi,
I have attached (as reply) two patches to one of the drivers in led
subsytem. The return value was not being checked before proceeding
leading to uninitialized values being passed in few places. In the
second patch, I am a bit unclear of the logic in the function being
patched. So modify it if required.
--------------------------
Raghavendra Prabhu
GPG Id : D72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977


[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] FIX: engine_state is uninitialized
  2011-04-16  8:37 [PATCH] Fix for unintialized variables Raghavendra D Prabhu
@ 2011-04-16  8:43 ` Raghavendra D Prabhu
  2011-04-18 11:21   ` Jonathan Neuschäfer
  2011-04-16  8:44 ` [PATCH 2/2] FIX: Check the return value before proceeding Raghavendra D Prabhu
  1 sibling, 1 reply; 4+ messages in thread
From: Raghavendra D Prabhu @ 2011-04-16  8:43 UTC (permalink / raw)
  To: Richard Purdie; +Cc: linux-kernel

In case lp5521_read() returns EIO, engine_state is in an uninitialized state. This
adds checks to fix that.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
  drivers/leds/leds-lp5521.c |   10 ++++++----
  1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index c0cff64..6c5eac0 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -176,11 +176,13 @@ static int lp5521_set_engine_mode(struct lp5521_engine *engine, u8 mode)
  
  	ret = lp5521_read(client, LP5521_REG_OP_MODE, &engine_state);
  
+	if (ret == 0){
  	/* set mode only for this engine */
-	engine_state &= ~(engine->engine_mask);
-	mode &= engine->engine_mask;
-	engine_state |= mode;
-	ret |= lp5521_write(client, LP5521_REG_OP_MODE, engine_state);
+		engine_state &= ~(engine->engine_mask);
+		mode &= engine->engine_mask;
+		engine_state |= mode;
+		ret |= lp5521_write(client, LP5521_REG_OP_MODE, engine_state);
+	}
  
  	return ret;
  }
-- 
1.7.4.4

--------------------------
Raghavendra Prabhu
GPG Id : D72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] FIX: engine_state is uninitialized
  2011-04-16  8:43 ` [PATCH 1/2] FIX: engine_state is uninitialized Raghavendra D Prabhu
@ 2011-04-18 11:21   ` Jonathan Neuschäfer
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Neuschäfer @ 2011-04-18 11:21 UTC (permalink / raw)
  To: Raghavendra D Prabhu; +Cc: Richard Purdie, linux-kernel

On Sat, Apr 16, 2011 at 02:13:13PM +0530, Raghavendra D Prabhu wrote:
> In case lp5521_read() returns EIO, engine_state is in an uninitialized state. This
> adds checks to fix that.
>
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> ---
>  drivers/leds/leds-lp5521.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index c0cff64..6c5eac0 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -176,11 +176,13 @@ static int lp5521_set_engine_mode(struct lp5521_engine *engine, u8 mode)
>  	ret = lp5521_read(client, LP5521_REG_OP_MODE, &engine_state);
> +	if (ret == 0){
>  	/* set mode only for this engine */
> -	engine_state &= ~(engine->engine_mask);
> -	mode &= engine->engine_mask;
> -	engine_state |= mode;
> -	ret |= lp5521_write(client, LP5521_REG_OP_MODE, engine_state);
> +		engine_state &= ~(engine->engine_mask);
> +		mode &= engine->engine_mask;
> +		engine_state |= mode;
> +		ret |= lp5521_write(client, LP5521_REG_OP_MODE, engine_state);
> +	}
>  	return ret;
>  }
> --
> 1.7.4.4

Usually something like

	if (ret < 0)
		goto out;

	/* some code here */

out:
	return ret;

is preferred to avoid unnecessary nesting/indentation. Anyway, thanks for the
patch (although I'm not the maintainer, but just commenting).

Thanks,
	Jonathan Neuschäfer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] FIX: Check the return value before proceeding
  2011-04-16  8:37 [PATCH] Fix for unintialized variables Raghavendra D Prabhu
  2011-04-16  8:43 ` [PATCH 1/2] FIX: engine_state is uninitialized Raghavendra D Prabhu
@ 2011-04-16  8:44 ` Raghavendra D Prabhu
  1 sibling, 0 replies; 4+ messages in thread
From: Raghavendra D Prabhu @ 2011-04-16  8:44 UTC (permalink / raw)
  To: Richard Purdie; +Cc: linux-kernel

In case of failure from lp5521_read, an uninitialized mode is passed to lp5521_write.
This adds checks to prevent that.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
  drivers/leds/leds-lp5521.c |   40 ++++++++++++++++++++++------------------
  1 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 6c5eac0..f03a524 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -176,7 +176,7 @@ static int lp5521_set_engine_mode(struct lp5521_engine *engine, u8 mode)
  
  	ret = lp5521_read(client, LP5521_REG_OP_MODE, &engine_state);
  
-	if (ret == 0){
+	if (!ret){
  	/* set mode only for this engine */
  		engine_state &= ~(engine->engine_mask);
  		mode &= engine->engine_mask;
@@ -192,6 +192,7 @@ static int lp5521_load_program(struct lp5521_engine *eng, const u8 *pattern)
  	struct lp5521_chip *chip = engine_to_lp5521(eng);
  	struct i2c_client *client = chip->client;
  	int ret;
+	int ret_read;
  	int addr;
  	u8 mode;
  
@@ -199,23 +200,26 @@ static int lp5521_load_program(struct lp5521_engine *eng, const u8 *pattern)
  	ret = lp5521_set_engine_mode(eng, LP5521_CMD_DIRECT);
  	/* Mode change requires min 500 us delay. 1 - 2 ms  with margin */
  	usleep_range(1000, 2000);
-	ret |= lp5521_read(client, LP5521_REG_OP_MODE, &mode);
-
-	/* For loading, all the engines to load mode */
-	lp5521_write(client, LP5521_REG_OP_MODE, LP5521_CMD_DIRECT);
-	/* Mode change requires min 500 us delay. 1 - 2 ms  with margin */
-	usleep_range(1000, 2000);
-	lp5521_write(client, LP5521_REG_OP_MODE, LP5521_CMD_LOAD);
-	/* Mode change requires min 500 us delay. 1 - 2 ms  with margin */
-	usleep_range(1000, 2000);
-
-	addr = LP5521_PROG_MEM_BASE + eng->prog_page * LP5521_PROG_MEM_SIZE;
-	i2c_smbus_write_i2c_block_data(client,
-				addr,
-				LP5521_PROG_MEM_SIZE,
-				pattern);
-
-	ret |= lp5521_write(client, LP5521_REG_OP_MODE, mode);
+	ret_read = lp5521_read(client, LP5521_REG_OP_MODE, &mode);
+	ret |= ret_read;
+
+	if (!ret_read){
+		/* For loading, all the engines to load mode */
+		lp5521_write(client, LP5521_REG_OP_MODE, LP5521_CMD_DIRECT);
+		/* Mode change requires min 500 us delay. 1 - 2 ms  with margin */
+		usleep_range(1000, 2000);
+		lp5521_write(client, LP5521_REG_OP_MODE, LP5521_CMD_LOAD);
+		/* Mode change requires min 500 us delay. 1 - 2 ms  with margin */
+		usleep_range(1000, 2000);
+
+		addr = LP5521_PROG_MEM_BASE + eng->prog_page * LP5521_PROG_MEM_SIZE;
+		i2c_smbus_write_i2c_block_data(client,
+					addr,
+					LP5521_PROG_MEM_SIZE,
+					pattern);
+
+		ret |= lp5521_write(client, LP5521_REG_OP_MODE, mode);
+	}
  	return ret;
  }
  
-- 
1.7.4.4

--------------------------
Raghavendra Prabhu
GPG Id : D72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-04-18 11:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-16  8:37 [PATCH] Fix for unintialized variables Raghavendra D Prabhu
2011-04-16  8:43 ` [PATCH 1/2] FIX: engine_state is uninitialized Raghavendra D Prabhu
2011-04-18 11:21   ` Jonathan Neuschäfer
2011-04-16  8:44 ` [PATCH 2/2] FIX: Check the return value before proceeding Raghavendra D Prabhu

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).