Re: [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code

[Laurent Vivier]

Le 25/09/2023 à 20:56, Thomas Huth a écrit :
> Rename the innermost variables to make the code compile
> without warnings when using -Wshadow=local.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/m68k/bootinfo.h      | 10 ++++------
>   disas/m68k.c            |  8 ++++----
>   target/m68k/translate.c |  8 ++++----
>   3 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
> index a3d37e3c80..d077d03559 100644
> --- a/hw/m68k/bootinfo.h
> +++ b/hw/m68k/bootinfo.h
> @@ -44,15 +44,14 @@
>   
>   #define BOOTINFOSTR(base, id, string) \
>       do { \
> -        int i; \
>           stw_p(base, id); \
>           base += 2; \
>           stw_p(base, \
>                    (sizeof(struct bi_record) + strlen(string) + \
>                     1 /* null termination */ + 3 /* padding */) & ~3); \
>           base += 2; \
> -        for (i = 0; string[i]; i++) { \
> -            stb_p(base++, string[i]); \
> +        for (int _i = 0; string[_i]; _i++) { \
> +            stb_p(base++, string[_i]); \
>           } \
>           stb_p(base++, 0); \
>           base = QEMU_ALIGN_PTR_UP(base, 4); \
> @@ -60,7 +59,6 @@
>   
>   #define BOOTINFODATA(base, id, data, len) \
>       do { \
> -        int i; \
>           stw_p(base, id); \
>           base += 2; \
>           stw_p(base, \
> @@ -69,8 +67,8 @@
>           base += 2; \
>           stw_p(base, len); \
>           base += 2; \
> -        for (i = 0; i < len; ++i) { \
> -            stb_p(base++, data[i]); \
> +        for (int _i = 0; _i < len; ++_i) { \
> +            stb_p(base++, data[_i]); \
>           } \
>           base = QEMU_ALIGN_PTR_UP(base, 4); \
>       } while (0)
> diff --git a/disas/m68k.c b/disas/m68k.c
> index aefaecfbd6..a384b4cb64 100644
> --- a/disas/m68k.c
> +++ b/disas/m68k.c
> @@ -1632,10 +1632,10 @@ print_insn_arg (const char *d,
>       case '2':
>       case '3':
>         {
> -       int val = fetch_arg (buffer, place, 5, info);
> +       int val2 = fetch_arg (buffer, place, 5, info);
>           const char *name = 0;
>   
> -	switch (val)
> +       switch (val2)
>           {
>           case 2: name = "%tt0"; break;
>           case 3: name = "%tt1"; break;
> @@ -1655,12 +1655,12 @@ print_insn_arg (const char *d,
>               int break_reg = ((buffer[3] >> 2) & 7);
>   
>   	      (*info->fprintf_func)
> -               (info->stream, val == 0x1c ? "%%bad%d" : "%%bac%d",
> +               (info->stream, val2 == 0x1c ? "%%bad%d" : "%%bac%d",
>                  break_reg);
>             }
>             break;
>           default:
> -           (*info->fprintf_func) (info->stream, "<mmu register %d>", val);
> +           (*info->fprintf_func) (info->stream, "<mmu register %d>", val2);
>           }
>         if (name)
>           (*info->fprintf_func) (info->stream, "%s", name);

"reg" would be a better name than "val2".

> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 9e224fe796..b28d7f7d4b 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -824,14 +824,14 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext 
> *s, int mode, int reg0,
>           reg = get_areg(s, reg0);
>           result = gen_ldst(s, opsize, reg, val, what, index);
>           if (what == EA_STORE || !addrp) {
> -            TCGv tmp = tcg_temp_new();
> +            TCGv tmp2 = tcg_temp_new();
>               if (reg0 == 7 && opsize == OS_BYTE &&
>                   m68k_feature(s->env, M68K_FEATURE_M68K)) {
> -                tcg_gen_addi_i32(tmp, reg, 2);
> +                tcg_gen_addi_i32(tmp2, reg, 2);
>               } else {
> -                tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
> +                tcg_gen_addi_i32(tmp2, reg, opsize_bytes(opsize));
>               }
> -            delay_set_areg(s, reg0, tmp, true);
> +            delay_set_areg(s, reg0, tmp2, true);
>           }
>           return result;
>       case 4: /* Indirect predecrememnt.  */

"inc" would be a better name than "val2".

Otherwise:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


Thanks,
Laurent