diff mbox series

m68k: Silence -Wshadow=local warnings in the m68k code

Message ID 20230925185603.106945-1-thuth@redhat.com
State New
Headers show
Series m68k: Silence -Wshadow=local warnings in the m68k code | expand

Commit Message

Thomas Huth Sept. 25, 2023, 6:56 p.m. UTC
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(-)

Comments

Laurent Vivier Sept. 26, 2023, 7:43 a.m. UTC | #1
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
Markus Armbruster Sept. 26, 2023, 12:19 p.m. UTC | #2
Thomas Huth <thuth@redhat.com> writes:

> Rename the innermost variables to make the code compile
> without warnings when using -Wshadow=local.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Clashes with patches from Philippe and Laurent:

    [PATCH v2 05/22] target/m68k: Clean up local variable shadowing
    [PATCH] disas/m68k: clean up local variable shadowing

You guys figure out how to combine them, please :)
Thomas Huth Sept. 26, 2023, 12:59 p.m. UTC | #3
On 26/09/2023 14.19, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Rename the innermost variables to make the code compile
>> without warnings when using -Wshadow=local.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> Clashes with patches from Philippe and Laurent:
> 
>      [PATCH v2 05/22] target/m68k: Clean up local variable shadowing
>      [PATCH] disas/m68k: clean up local variable shadowing
> 
> You guys figure out how to combine them, please :)

Ok, then never mind about my patch.
Anyway, it's getting confusing what has already been fixed and what not ... 
could you please pick up all patches that are available so far and send a 
pull request for them?

  Thanks,
   Thomas
Markus Armbruster Sept. 26, 2023, 1:15 p.m. UTC | #4
Thomas Huth <thuth@redhat.com> writes:

> On 26/09/2023 14.19, Markus Armbruster wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> Rename the innermost variables to make the code compile
>>> without warnings when using -Wshadow=local.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> Clashes with patches from Philippe and Laurent:
>>
>>      [PATCH v2 05/22] target/m68k: Clean up local variable shadowing
>>      [PATCH] disas/m68k: clean up local variable shadowing
>>
>> You guys figure out how to combine them, please :)
>
> Ok, then never mind about my patch.
> Anyway, it's getting confusing what has already been fixed and what not ... could you please pick up all patches that are available so far and send a pull request for them?

I'm collecting at https://repo.or.cz/qemu/armbru.git in branch
shadow-next.  I'll send a new summary shortly.  I intend to send a PR,
but first I need to collect R-bys and drop patches that aren't ready, if
any.
diff mbox series

Patch

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);
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.  */