qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code
@ 2023-09-25 18:56 Thomas Huth
  2023-09-26  7:43 ` Laurent Vivier
  2023-09-26 12:19 ` Markus Armbruster
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Huth @ 2023-09-25 18:56 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel; +Cc: Markus Armbruster

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code
  2023-09-25 18:56 [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code Thomas Huth
@ 2023-09-26  7:43 ` Laurent Vivier
  2023-09-26 12:19 ` Markus Armbruster
  1 sibling, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2023-09-26  7:43 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: Markus Armbruster

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code
  2023-09-25 18:56 [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code Thomas Huth
  2023-09-26  7:43 ` Laurent Vivier
@ 2023-09-26 12:19 ` Markus Armbruster
  2023-09-26 12:59   ` Thomas Huth
  1 sibling, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2023-09-26 12:19 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Laurent Vivier, qemu-devel, Philippe Mathieu-Daudé

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code
  2023-09-26 12:19 ` Markus Armbruster
@ 2023-09-26 12:59   ` Thomas Huth
  2023-09-26 13:15     ` Markus Armbruster
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2023-09-26 12:59 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Laurent Vivier, qemu-devel, Philippe Mathieu-Daudé

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code
  2023-09-26 12:59   ` Thomas Huth
@ 2023-09-26 13:15     ` Markus Armbruster
  0 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2023-09-26 13:15 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Laurent Vivier, qemu-devel, Philippe Mathieu-Daudé

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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-09-26 13:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-25 18:56 [PATCH] m68k: Silence -Wshadow=local warnings in the m68k code Thomas Huth
2023-09-26  7:43 ` Laurent Vivier
2023-09-26 12:19 ` Markus Armbruster
2023-09-26 12:59   ` Thomas Huth
2023-09-26 13:15     ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).