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

On 2017/12/7 17:03, Arnd Bergmann wrote:

On Thu, Dec 7, 2017 at 9:16 AM, Xiongfeng Wang wangxiongfeng2@huawei.com wrote:

...

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 wangxiongfeng2@huawei.com

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.

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

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

...

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.

...

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

}

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

     Arnd

.