[PATCH] auxdisplay: img-ascii-lcd: fix maybe-uninitialized warning

Discussion:

Xiongfeng Wang

2017-12-07 08:16:29 UTC

gcc prints the following warning:
drivers/auxdisplay/img-ascii-lcd.c: In function ‘malta_update’:
drivers/auxdisplay/img-ascii-lcd.c:109: warning: ‘err’ may be usedun initialized in this function
drivers/auxdisplay/img-ascii-lcd.c: In function ‘sead3_update’:
drivers/auxdisplay/img-ascii-lcd.c:207: warning: ‘err’ may be used uninitialized in this function

When ctx->cfg->num_chars is zero, there will be a false error info
printed. Fix this by recontruct the code and initializing the variable
'err' to zero.

Signed-off-by: Xiongfeng Wang <***@huawei.com>
---
drivers/auxdisplay/img-ascii-lcd.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index db040b3..15048c1 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -102,12 +102,11 @@ static void malta_update(struct img_ascii_lcd_ctx *ctx)
for (i = 0; i < ctx->cfg->num_chars; i++) {
err = regmap_write(ctx->regmap,
ctx->offset + (i * 8), ctx->curr[i]);
- if (err)
- break;
+ if (err) {
+ pr_err_ratelimited("Failed to update LCD display: %d\n", err);
+ return;
+ }
}
-
- if (unlikely(err))
- pr_err_ratelimited("Failed to update LCD display: %d\n", err);
}

static struct img_ascii_lcd_config malta_config = {
@@ -180,32 +179,32 @@ static int sead3_wait_lcd_idle(struct img_ascii_lcd_ctx *ctx)
static void sead3_update(struct img_ascii_lcd_ctx *ctx)
{
unsigned int i;
- int err;
+ int err = 0;

for (i = 0; i < ctx->cfg->num_chars; i++) {
err = sead3_wait_lcd_idle(ctx);
if (err)
- break;
+ goto out_err;

err = regmap_write(ctx->regmap,
ctx->offset + SEAD3_REG_LCD_CTRL,
SEAD3_REG_LCD_CTRL_SETDRAM | i);
if (err)
- break;
+ goto out_err;

err = sead3_wait_lcd_idle(ctx);
if (err)
- break;
+ goto out_err;

err = regmap_write(ctx->regmap,
ctx->offset + SEAD3_REG_LCD_DATA,
ctx->curr[i]);
if (err)
- break;
+ goto out_err;
}

- if (unlikely(err))
- pr_err_ratelimited("Failed to update LCD display: %d\n", err);
+out_err:
+ pr_err_ratelimited("Failed to update LCD display: %d\n", err);
}

static struct img_ascii_lcd_config sead3_config = {

--
1.7.12.4

Arnd Bergmann

2017-12-07 09:03:31 UTC

Permalink

On Thu, Dec 7, 2017 at 9:16 AM, Xiongfeng Wang

Post by Xiongfeng Wang
drivers/auxdisplay/img-ascii-lcd.c:109: warning: ‘err’ may be usedun initialized in this function
drivers/auxdisplay/img-ascii-lcd.c:207: warning: ‘err’ may be used uninitialized in this function
When ctx->cfg->num_chars is zero, there will be a false error info
printed. Fix this by recontruct the code and initializing the variable
'err' to zero.

I wonder how you ran into this. I'm not seeing that warning on my machine,
and I thought I had fixed all '-Wmaybe-uninitialized' warnings. Before you
send it to the maintainer, I'd try to figure out what the difference is between
our setups.

Do you get other -Wmaybe-uninitialized warnings in the same build, or is this
the only one?

Post by Xiongfeng Wang
drivers/auxdisplay/img-ascii-lcd.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/auxdisplay/img-ascii-lcd.c b/drivers/auxdisplay/img-ascii-lcd.c
index db040b3..15048c1 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -102,12 +102,11 @@ static void malta_update(struct img_ascii_lcd_ctx *ctx)
for (i = 0; i < ctx->cfg->num_chars; i++) {
err = regmap_write(ctx->regmap,
ctx->offset + (i * 8), ctx->curr[i]);
- if (err)
- break;
+ if (err) {
+ pr_err_ratelimited("Failed to update LCD display: %d\n", err);
+ return;
+ }
}
-
- if (unlikely(err))
- pr_err_ratelimited("Failed to update LCD display: %d\n", err);
}

This part looks good.

Post by Xiongfeng Wang
static struct img_ascii_lcd_config malta_config = {
@@ -180,32 +179,32 @@ static int sead3_wait_lcd_idle(struct img_ascii_lcd_ctx *ctx)
static void sead3_update(struct img_ascii_lcd_ctx *ctx)
{
unsigned int i;
- int err;
+ int err = 0;
for (i = 0; i < ctx->cfg->num_chars; i++) {
err = sead3_wait_lcd_idle(ctx);
if (err)
- break;
+ goto out_err;
err = regmap_write(ctx->regmap,
ctx->offset + SEAD3_REG_LCD_CTRL,
SEAD3_REG_LCD_CTRL_SETDRAM | i);
if (err)
- break;
+ goto out_err;
err = sead3_wait_lcd_idle(ctx);
if (err)
- break;
+ goto out_err;
err = regmap_write(ctx->regmap,
ctx->offset + SEAD3_REG_LCD_DATA,
ctx->curr[i]);
if (err)
- break;
+ goto out_err;
}
- if (unlikely(err))
- pr_err_ratelimited("Failed to update LCD display: %d\n", err);
+ pr_err_ratelimited("Failed to update LCD display: %d\n", err);
}

I think you forgot a 'return' statement befor ethe label here.

Arnd

Xiongfeng Wang

2017-12-11 05:47:45 UTC

Permalink

Post by Arnd Bergmann
On Thu, Dec 7, 2017 at 9:16 AM, Xiongfeng Wang

Sorry, I forget the designate the 'ARCH' I used. So it's x86 compiler that prints these warnings.
ARM64 compiler won't report these kind of warnings.

Since 'num_chars'is guaranteed to be a positive number, this is not a real bug.
Please just ignore this patch.

Thanks,
Xiongfeng Wang

Post by Arnd Bergmann
Do you get other -Wmaybe-uninitialized warnings in the same build, or is this
the only one?

This part looks good.

I think you forgot a 'return' statement befor ethe label here.
Arnd
.