On Thu, Dec 7, 2017 at 9:16 AM, Xiongfeng Wang
<wangxiongfe...@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 <wangxiongfe...@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.

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
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to