On Thu, Dec 7, 2017 at 9:16 AM, Xiongfeng Wang
<[email protected]> 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 <[email protected]>
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?
> 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
_______________________________________________
linaro-dev mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/linaro-dev