[PATCH 00/11] (few more) Steps towards enabling -Wshadow

[PATCH 00/11] (few more) Steps towards enabling -Wshadow

[Philippe Mathieu-Daudé]

For rational see Markus cover on
https://lore.kernel.org/qemu-devel/20230831132546.3525721-1-armbru@redhat.com/

This series contains few more, my take.

Based-on: <20230831132546.3525721-1-armbru@redhat.com>

Philippe Mathieu-Daudé (11):
  tcg: Clean up local variable shadowing
  target/arm: Clean up local variable shadowing
  target/mips: Clean up local variable shadowing
  target/m68k: Clean up local variable shadowing
  hw/arm/virt: Clean up local variable shadowing
  hw/arm/allwinner: Clean up local variable shadowing
  hw/arm/aspeed: Clean up local variable shadowing
  hw/m68k: Clean up local variable shadowing
  hw/ide/ahci: Clean up local variable shadowing
  net/eth: Clean up local variable shadowing
  sysemu/device_tree: Clean up local variable shadowing

 hw/m68k/bootinfo.h                       | 10 ++++------
 include/sysemu/device_tree.h             |  6 ++----
 accel/tcg/tb-maint.c                     |  3 +--
 hw/arm/allwinner-r40.c                   |  7 +++----
 hw/arm/aspeed_ast2600.c                  |  2 +-
 hw/arm/virt.c                            |  3 +--
 hw/ide/ahci.c                            |  6 ++----
 net/eth.c                                |  2 +-
 target/arm/hvf/hvf.c                     |  1 -
 target/arm/tcg/mve_helper.c              |  8 ++++----
 target/arm/tcg/translate-m-nocp.c        |  2 +-
 target/m68k/translate.c                  |  2 +-
 target/mips/tcg/msa_helper.c             |  8 ++++----
 target/mips/tcg/translate.c              |  8 +++-----
 tcg/tcg.c                                | 16 ++++++++--------
 target/mips/tcg/nanomips_translate.c.inc |  6 +++---
 16 files changed, 39 insertions(+), 51 deletions(-)

-- 
2.41.0

[PATCH 01/11] tcg: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  tcg/tcg.c:2551:27: error: declaration shadows a local variable [-Werror,-Wshadow]
                    MemOp op = get_memop(oi);
                          ^
  tcg/tcg.c:2437:12: note: previous declaration is here
    TCGOp *op;
           ^
  accel/tcg/tb-maint.c:245:18: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (int i = 0; i < V_L2_SIZE; i++) {
                 ^
  accel/tcg/tb-maint.c:210:9: note: previous declaration is here
    int i;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tb-maint.c |  3 +--
 tcg/tcg.c            | 16 ++++++++--------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index c406b2f7b7..f1cf3ad736 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -207,13 +207,12 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, bool alloc)
 {
     PageDesc *pd;
     void **lp;
-    int i;
 
     /* Level 1.  Always allocated.  */
     lp = l1_map + ((index >> v_l1_shift) & (v_l1_size - 1));
 
     /* Level 2..N-1.  */
-    for (i = v_l2_levels; i > 0; i--) {
+    for (int i = v_l2_levels; i > 0; i--) {
         void **p = qatomic_rcu_read(lp);
 
         if (p == NULL) {
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 620dbe08da..46a3a231b8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2548,21 +2548,21 @@ static void tcg_dump_ops(TCGContext *s, FILE *f, bool have_prefs)
                 {
                     const char *s_al, *s_op, *s_at;
                     MemOpIdx oi = op->args[k++];
-                    MemOp op = get_memop(oi);
+                    MemOp o = get_memop(oi);
                     unsigned ix = get_mmuidx(oi);
 
-                    s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
-                    s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
-                    s_at = atom_name[(op & MO_ATOM_MASK) >> MO_ATOM_SHIFT];
-                    op &= ~(MO_AMASK | MO_BSWAP | MO_SSIZE | MO_ATOM_MASK);
+                    s_al = alignment_name[(o & MO_AMASK) >> MO_ASHIFT];
+                    s_op = ldst_name[o & (MO_BSWAP | MO_SSIZE)];
+                    s_at = atom_name[(o & MO_ATOM_MASK) >> MO_ATOM_SHIFT];
+                    o &= ~(MO_AMASK | MO_BSWAP | MO_SSIZE | MO_ATOM_MASK);
 
                     /* If all fields are accounted for, print symbolically. */
-                    if (!op && s_al && s_op && s_at) {
+                    if (!o && s_al && s_op && s_at) {
                         col += ne_fprintf(f, ",%s%s%s,%u",
                                           s_at, s_al, s_op, ix);
                     } else {
-                        op = get_memop(oi);
-                        col += ne_fprintf(f, ",$0x%x,%u", op, ix);
+                        o = get_memop(oi);
+                        col += ne_fprintf(f, ",$0x%x,%u", o, ix);
                     }
                     i = 1;
                 }
-- 
2.41.0

[PATCH 02/11] target/arm: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  target/arm/tcg/translate-m-nocp.c:509:18: error: declaration shadows a local variable [-Werror,-Wshadow]
        TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
                 ^
  target/arm/tcg/translate-m-nocp.c:433:14: note: previous declaration is here
    TCGv_i32 tmp;
             ^
  target/arm/tcg/mve_helper.c:2463:17: error: declaration shadows a local variable [-Werror,-Wshadow]
        int64_t extval = sextract64(src << shift, 0, 48);
                ^
  target/arm/tcg/mve_helper.c:2443:18: note: previous declaration is here
    int64_t val, extval;
                 ^
  target/arm/hvf/hvf.c:1936:13: error: declaration shadows a local variable [-Werror,-Wshadow]
        int ret = 0;
            ^
  target/arm/hvf/hvf.c:1807:9: note: previous declaration is here
    int ret;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c              | 1 -
 target/arm/tcg/mve_helper.c       | 8 ++++----
 target/arm/tcg/translate-m-nocp.c | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 486f90be1d..20d534faef 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1933,7 +1933,6 @@ int hvf_vcpu_exec(CPUState *cpu)
         uint32_t rt = (syndrome >> 5) & 0x1f;
         uint32_t reg = syndrome & SYSREG_MASK;
         uint64_t val;
-        int ret = 0;
 
         if (isread) {
             ret = hvf_sysreg_read(cpu, reg, rt);
diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c
index 403b345ea3..32087b6f0a 100644
--- a/target/arm/tcg/mve_helper.c
+++ b/target/arm/tcg/mve_helper.c
@@ -924,8 +924,8 @@ DO_1OP_IMM(vorri, DO_ORRI)
         bool qc = false;                                                \
         for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {              \
             bool sat = false;                                           \
-            TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);          \
-            mergemask(&d[H##ESIZE(e)], r, mask);                        \
+            TYPE r_ = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);         \
+            mergemask(&d[H##ESIZE(e)], r_, mask);                       \
             qc |= sat & mask & 1;                                       \
         }                                                               \
         if (qc) {                                                       \
@@ -2460,7 +2460,7 @@ static inline int64_t do_sqrshl48_d(int64_t src, int64_t shift,
             return extval;
         }
     } else if (shift < 48) {
-        int64_t extval = sextract64(src << shift, 0, 48);
+        extval = sextract64(src << shift, 0, 48);
         if (!sat || src == (extval >> shift)) {
             return extval;
         }
@@ -2492,7 +2492,7 @@ static inline uint64_t do_uqrshl48_d(uint64_t src, int64_t shift,
             return extval;
         }
     } else if (shift < 48) {
-        uint64_t extval = extract64(src << shift, 0, 48);
+        extval = extract64(src << shift, 0, 48);
         if (!sat || src == (extval >> shift)) {
             return extval;
         }
diff --git a/target/arm/tcg/translate-m-nocp.c b/target/arm/tcg/translate-m-nocp.c
index 33f6478bb9..42308c4db5 100644
--- a/target/arm/tcg/translate-m-nocp.c
+++ b/target/arm/tcg/translate-m-nocp.c
@@ -506,7 +506,7 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
 
         gen_branch_fpInactive(s, TCG_COND_EQ, lab_active);
         /* fpInactive case: reads as FPDSCR_NS */
-        TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
+        tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
         storefn(s, opaque, tmp, true);
         lab_end = gen_new_label();
         tcg_gen_br(lab_end);
-- 
2.41.0

[PATCH 03/11] target/mips: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  target/mips/tcg/nanomips_translate.c.inc:4410:33: error: declaration shadows a local variable [-Werror,-Wshadow]
                        int32_t imm = extract32(ctx->opcode, 1, 13) |
                                ^
  target/mips/tcg/nanomips_translate.c.inc:3577:9: note: previous declaration is here
    int imm;
        ^
  target/mips/tcg/translate.c:15578:19: error: declaration shadows a local variable [-Werror,-Wshadow]
    for (unsigned i = 1; i < 32; i++) {
                  ^
  target/mips/tcg/translate.c:15567:9: note: previous declaration is here
    int i;
        ^
  target/mips/tcg/msa_helper.c:7478:13: error: declaration shadows a local variable [-Werror,-Wshadow]
            MSA_FLOAT_MAXOP(pwx->w[0], min, pws->w[0], pws->w[0], 32);
            ^
  target/mips/tcg/msa_helper.c:7434:23: note: expanded from macro 'MSA_FLOAT_MAXOP'
        float_status *status = &env->active_tc.msa_fp_status;
                      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/msa_helper.c             | 8 ++++----
 target/mips/tcg/translate.c              | 8 +++-----
 target/mips/tcg/nanomips_translate.c.inc | 6 +++---
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/target/mips/tcg/msa_helper.c b/target/mips/tcg/msa_helper.c
index 29b31d70fe..391a5fca26 100644
--- a/target/mips/tcg/msa_helper.c
+++ b/target/mips/tcg/msa_helper.c
@@ -7431,15 +7431,15 @@ void helper_msa_ftq_df(CPUMIPSState *env, uint32_t df, uint32_t wd,
 
 #define MSA_FLOAT_MAXOP(DEST, OP, ARG1, ARG2, BITS)                         \
     do {                                                                    \
-        float_status *status = &env->active_tc.msa_fp_status;               \
+        float_status *status_ = &env->active_tc.msa_fp_status;              \
         int c;                                                              \
                                                                             \
-        set_float_exception_flags(0, status);                               \
-        DEST = float ## BITS ## _ ## OP(ARG1, ARG2, status);                \
+        set_float_exception_flags(0, status_);                              \
+        DEST = float ## BITS ## _ ## OP(ARG1, ARG2, status_);               \
         c = update_msacsr(env, 0, 0);                                       \
                                                                             \
         if (get_enabled_exceptions(env, c)) {                               \
-            DEST = ((FLOAT_SNAN ## BITS(status) >> 6) << 6) | c;            \
+            DEST = ((FLOAT_SNAN ## BITS(status_) >> 6) << 6) | c;           \
         }                                                                   \
     } while (0)
 
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 9bb40f1849..26d741d960 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15564,10 +15564,8 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
 
 void mips_tcg_init(void)
 {
-    int i;
-
     cpu_gpr[0] = NULL;
-    for (i = 1; i < 32; i++)
+    for (unsigned i = 1; i < 32; i++)
         cpu_gpr[i] = tcg_global_mem_new(cpu_env,
                                         offsetof(CPUMIPSState,
                                                  active_tc.gpr[i]),
@@ -15584,7 +15582,7 @@ void mips_tcg_init(void)
                                                rname);
     }
 #endif /* !TARGET_MIPS64 */
-    for (i = 0; i < 32; i++) {
+    for (unsigned i = 0; i < 32; i++) {
         int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
 
         fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
@@ -15592,7 +15590,7 @@ void mips_tcg_init(void)
     msa_translate_init();
     cpu_PC = tcg_global_mem_new(cpu_env,
                                 offsetof(CPUMIPSState, active_tc.PC), "PC");
-    for (i = 0; i < MIPS_DSP_ACC; i++) {
+    for (unsigned i = 0; i < MIPS_DSP_ACC; i++) {
         cpu_HI[i] = tcg_global_mem_new(cpu_env,
                                        offsetof(CPUMIPSState, active_tc.HI[i]),
                                        regnames_HI[i]);
diff --git a/target/mips/tcg/nanomips_translate.c.inc b/target/mips/tcg/nanomips_translate.c.inc
index a98dde0d2e..d81a7c2d11 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -4407,8 +4407,8 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx)
                 case NM_BPOSGE32C:
                     check_dsp_r3(ctx);
                     {
-                        int32_t imm = extract32(ctx->opcode, 1, 13) |
-                                      extract32(ctx->opcode, 0, 1) << 13;
+                        imm = extract32(ctx->opcode, 1, 13)
+                            | extract32(ctx->opcode, 0, 1) << 13;
 
                         gen_compute_branch_nm(ctx, OPC_BPOSGE32, 4, -1, -2,
                                               imm << 1);
@@ -4635,7 +4635,7 @@ static int decode_isa_nanomips(CPUMIPSState *env, DisasContext *ctx)
         break;
     case NM_LI16:
         {
-            int imm = extract32(ctx->opcode, 0, 7);
+            imm = extract32(ctx->opcode, 0, 7);
             imm = (imm == 0x7f ? -1 : imm);
             if (rt != 0) {
                 tcg_gen_movi_tl(cpu_gpr[rt], imm);
-- 
2.41.0

[PATCH 04/11] target/m68k: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  target/m68k/translate.c:828:18: error: declaration shadows a local variable [-Werror,-Wshadow]
            TCGv tmp = tcg_temp_new();
                 ^
  target/m68k/translate.c:801:15: note: previous declaration is here
    TCGv reg, tmp, result;
              ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/m68k/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 15b3701b8f..f69ae0e3b0 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -825,7 +825,7 @@ 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();
+            tmp = tcg_temp_new();
             if (reg0 == 7 && opsize == OS_BYTE &&
                 m68k_feature(s->env, M68K_FEATURE_M68K)) {
                 tcg_gen_addi_i32(tmp, reg, 2);
-- 
2.41.0

[PATCH 05/11] hw/arm/virt: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/arm/virt.c:821:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            qemu_irq irq = qdev_get_gpio_in(vms->gic,
                     ^
  hw/arm/virt.c:803:13: note: previous declaration is here
        int irq;
            ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a13c658bbf..32a964b572 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -800,7 +800,6 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
     for (i = 0; i < smp_cpus; i++) {
         DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
         int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
-        int irq;
         /* Mapping from the output timer irq lines from the CPU to the
          * GIC PPI inputs we use for the virt board.
          */
@@ -811,7 +810,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
             [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
         };
 
-        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+        for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
             qdev_connect_gpio_out(cpudev, irq,
                                   qdev_get_gpio_in(vms->gic,
                                                    ppibase + timer_irq[irq]));
-- 
2.41.0

[PATCH 06/11] hw/arm/allwinner: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/arm/allwinner-r40.c:412:14: error: declaration shadows a local variable [-Werror,-Wshadow]
    for (int i = 0; i < AW_R40_NUM_MMCS; i++) {
             ^
  hw/arm/allwinner-r40.c:299:14: note: previous declaration is here
    unsigned i;
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/allwinner-r40.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index 7d29eb224f..a0d367c60d 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -296,10 +296,9 @@ static void allwinner_r40_realize(DeviceState *dev, Error **errp)
 {
     const char *r40_nic_models[] = { "gmac", "emac", NULL };
     AwR40State *s = AW_R40(dev);
-    unsigned i;
 
     /* CPUs */
-    for (i = 0; i < AW_R40_NUM_CPUS; i++) {
+    for (unsigned i = 0; i < AW_R40_NUM_CPUS; i++) {
 
         /*
          * Disable secondary CPUs. Guest EL3 firmware will start
@@ -335,7 +334,7 @@ static void allwinner_r40_realize(DeviceState *dev, Error **errp)
      * maintenance interrupt signal to the appropriate GIC PPI inputs,
      * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
      */
-    for (i = 0; i < AW_R40_NUM_CPUS; i++) {
+    for (unsigned i = 0; i < AW_R40_NUM_CPUS; i++) {
         DeviceState *cpudev = DEVICE(&s->cpus[i]);
         int ppibase = AW_R40_GIC_NUM_SPI + i * GIC_INTERNAL + GIC_NR_SGIS;
         int irq;
@@ -494,7 +493,7 @@ static void allwinner_r40_realize(DeviceState *dev, Error **errp)
                        qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_EMAC));
 
     /* Unimplemented devices */
-    for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
+    for (unsigned i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
         create_unimplemented_device(r40_unimplemented[i].device_name,
                                     r40_unimplemented[i].base,
                                     r40_unimplemented[i].size);
-- 
2.41.0

[PATCH 07/11] hw/arm/aspeed: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/arm/aspeed_ast2600.c:391:18: error: declaration shadows a local variable [-Werror,-Wshadow]
        qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
                 ^
  hw/arm/aspeed_ast2600.c:283:14: note: previous declaration is here
    qemu_irq irq;
             ^
  hw/arm/aspeed_ast2600.c:416:18: error: declaration shadows a local variable [-Werror,-Wshadow]
        qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
                 ^
  hw/arm/aspeed_ast2600.c:283:14: note: previous declaration is here
    qemu_irq irq;
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/aspeed_ast2600.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a..f54063445d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -280,7 +280,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     Error *err = NULL;
-    qemu_irq irq;
     g_autofree char *sram_name = NULL;
 
     /* Default boot region (SPI memory or ROMs) */
@@ -339,6 +338,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->num_cpus; i++) {
         SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
         DeviceState  *d   = DEVICE(&s->cpu[i]);
+        qemu_irq irq;
 
         irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
         sysbus_connect_irq(sbd, i, irq);
-- 
2.41.0

[PATCH 08/11] hw/m68k: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/m68k/virt.c:263:13: error: declaration shadows a local variable [-Werror,-Wshadow]
            BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
            ^
  hw/m68k/bootinfo.h:47:13: note: expanded from macro 'BOOTINFOSTR'
        int i; \
            ^
  hw/m68k/virt.c:130:9: note: previous declaration is here
    int i;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/bootinfo.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index a3d37e3c80..0e6e3eea87 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 (unsigned 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 (unsigned i_ = 0; i_ < len; ++i_) { \
+            stb_p(base++, data[i_]); \
         } \
         base = QEMU_ALIGN_PTR_UP(base, 4); \
     } while (0)
-- 
2.41.0

[PATCH 09/11] hw/ide/ahci: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

  hw/ide/ahci.c:1577:23: error: declaration shadows a local variable [-Werror,-Wshadow]
            IDEState *s = &ad->port.ifs[j];
                      ^
  hw/ide/ahci.c:1569:29: note: previous declaration is here
    void ahci_uninit(AHCIState *s)
                                ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 48d550f633..8c9a7c2117 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1573,10 +1573,8 @@ void ahci_uninit(AHCIState *s)
     for (i = 0; i < s->ports; i++) {
         AHCIDevice *ad = &s->dev[i];
 
-        for (j = 0; j < 2; j++) {
-            IDEState *s = &ad->port.ifs[j];
-
-            ide_exit(s);
+        for (j = 0; j < ARRAY_SIZE(ad->port.ifs); j++) {
+            ide_exit(&ad->port.ifs[j]);
         }
         object_unparent(OBJECT(&ad->port));
     }
-- 
2.41.0

[PATCH 10/11] net/eth: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  net/eth.c:435:20: error: declaration shadows a local variable [-Werror,-Wshadow]
            size_t input_size = iov_size(pkt, pkt_frags);
                   ^
  net/eth.c:413:16: note: previous declaration is here
        size_t input_size = iov_size(pkt, pkt_frags);
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 net/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/eth.c b/net/eth.c
index 649e66bb1f..cf030eed7b 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -432,7 +432,7 @@ _eth_get_rss_ex_src_addr(const struct iovec *pkt, int pkt_frags,
         }
 
         if (opthdr.type == IP6_OPT_HOME) {
-            size_t input_size = iov_size(pkt, pkt_frags);
+            input_size = iov_size(pkt, pkt_frags);
 
             if (input_size < opt_offset + sizeof(opthdr)) {
                 return false;
-- 
2.41.0

[PATCH 11/11] sysemu/device_tree: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/mips/boston.c:472:5: error: declaration shadows a local variable [-Werror,-Wshadow]
    qemu_fdt_setprop_cells(fdt, name, "reg", reg_base, reg_size);
    ^
  include/sysemu/device_tree.h:129:13: note: expanded from macro 'qemu_fdt_setprop_cells'
        int i;
            ^
  hw/mips/boston.c:461:9: note: previous declaration is here
    int i;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/device_tree.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ca5339beae..8eab395934 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -126,10 +126,8 @@ int qemu_fdt_add_path(void *fdt, const char *path);
 #define qemu_fdt_setprop_cells(fdt, node_path, property, ...)                 \
     do {                                                                      \
         uint32_t qdt_tmp[] = { __VA_ARGS__ };                                 \
-        int i;                                                                \
-                                                                              \
-        for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) {                           \
-            qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]);                             \
+        for (unsigned i_ = 0; i_ < ARRAY_SIZE(qdt_tmp); i_++) {               \
+            qdt_tmp[i_] = cpu_to_be32(qdt_tmp[i_]);                           \
         }                                                                     \
         qemu_fdt_setprop(fdt, node_path, property, qdt_tmp,                   \
                          sizeof(qdt_tmp));                                    \
-- 
2.41.0

Re: [PATCH 01/11] tcg: Clean up local variable shadowing

[Richard Henderson]

On 8/31/23 15:55, Philippe Mathieu-Daudé wrote:
> -                    MemOp op = get_memop(oi);
> +                    MemOp o = get_memop(oi);

mop would be a more descriptive replacement.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 07/11] hw/arm/aspeed: Clean up local variable shadowing

[Cédric Le Goater]

On 9/1/23 00:56, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    hw/arm/aspeed_ast2600.c:391:18: error: declaration shadows a local variable [-Werror,-Wshadow]
>          qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
>                   ^
>    hw/arm/aspeed_ast2600.c:283:14: note: previous declaration is here
>      qemu_irq irq;
>               ^
>    hw/arm/aspeed_ast2600.c:416:18: error: declaration shadows a local variable [-Werror,-Wshadow]
>          qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
>                   ^
>    hw/arm/aspeed_ast2600.c:283:14: note: previous declaration is here
>      qemu_irq irq;
>               ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/arm/aspeed_ast2600.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index a8b3a8065a..f54063445d 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -280,7 +280,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       AspeedSoCState *s = ASPEED_SOC(dev);
>       AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>       Error *err = NULL;
> -    qemu_irq irq;
>       g_autofree char *sram_name = NULL;
>   
>       /* Default boot region (SPI memory or ROMs) */
> @@ -339,6 +338,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>       for (i = 0; i < sc->num_cpus; i++) {
>           SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
>           DeviceState  *d   = DEVICE(&s->cpu[i]);
> +        qemu_irq irq;
>   
>           irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
>           sysbus_connect_irq(sbd, i, irq);

Re: [PATCH 10/11] net/eth: Clean up local variable shadowing

[Akihiko Odaki]

On 2023/09/01 7:56, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    net/eth.c:435:20: error: declaration shadows a local variable [-Werror,-Wshadow]
>              size_t input_size = iov_size(pkt, pkt_frags);
>                     ^
>    net/eth.c:413:16: note: previous declaration is here
>          size_t input_size = iov_size(pkt, pkt_frags);
>                 ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   net/eth.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/eth.c b/net/eth.c
> index 649e66bb1f..cf030eed7b 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -432,7 +432,7 @@ _eth_get_rss_ex_src_addr(const struct iovec *pkt, int pkt_frags,
>           }
>   
>           if (opthdr.type == IP6_OPT_HOME) {
> -            size_t input_size = iov_size(pkt, pkt_frags);
> +            input_size = iov_size(pkt, pkt_frags);

You can just remove this statement.

>   
>               if (input_size < opt_offset + sizeof(opthdr)) {
>                   return false;

Re: [PATCH 00/11] (few more) Steps towards enabling -Wshadow

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> For rational see Markus cover on
> https://lore.kernel.org/qemu-devel/20230831132546.3525721-1-armbru@redhat.com/
>
> This series contains few more, my take.
>
> Based-on: <20230831132546.3525721-1-armbru@redhat.com>

Awesome, thanks!

Re: [PATCH 02/11] target/arm: Clean up local variable shadowing

[Peter Maydell]

On Thu, 31 Aug 2023 at 23:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Fix:
>
>   target/arm/tcg/translate-m-nocp.c:509:18: error: declaration shadows a local variable [-Werror,-Wshadow]
>         TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
>                  ^
>   target/arm/tcg/translate-m-nocp.c:433:14: note: previous declaration is here
>     TCGv_i32 tmp;
>              ^
>   target/arm/tcg/mve_helper.c:2463:17: error: declaration shadows a local variable [-Werror,-Wshadow]
>         int64_t extval = sextract64(src << shift, 0, 48);
>                 ^
>   target/arm/tcg/mve_helper.c:2443:18: note: previous declaration is here
>     int64_t val, extval;
>                  ^
>   target/arm/hvf/hvf.c:1936:13: error: declaration shadows a local variable [-Werror,-Wshadow]
>         int ret = 0;
>             ^
>   target/arm/hvf/hvf.c:1807:9: note: previous declaration is here
>     int ret;
>         ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/arm/hvf/hvf.c              | 1 -
>  target/arm/tcg/mve_helper.c       | 8 ++++----
>  target/arm/tcg/translate-m-nocp.c | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 486f90be1d..20d534faef 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1933,7 +1933,6 @@ int hvf_vcpu_exec(CPUState *cpu)
>          uint32_t rt = (syndrome >> 5) & 0x1f;
>          uint32_t reg = syndrome & SYSREG_MASK;
>          uint64_t val;
> -        int ret = 0;
>
>          if (isread) {
>              ret = hvf_sysreg_read(cpu, reg, rt);

I'm not sure this is correct.

The hvf_vcpu_exec() function is not documented, but in practice
its caller expects it to return either EXCP_DEBUG (for "this was
a guest debug exception you need to deal with") or something else
(presumably the intention being 0 for OK).

The hvf_sysreg_read() and hvf_sysreg_write() functions are also not
documented, but they return 0 on success, or 1 for a completely
unrecognized sysreg where we've raised the UNDEF exception (but
not if we raised an UNDEF exception for an unrecognized GIC sysreg --
I think this is a bug). We use this return value to decide whether
we need to advance the PC past the insn or not. It's not the same
as the return value we want to return from hvf_vcpu_exec().

So I think the correct fix here is to retain the variable as
locally scoped but give it a name that doesn't clash with the
other function-scoped variable.

> diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c
> index 403b345ea3..32087b6f0a 100644
> --- a/target/arm/tcg/mve_helper.c
> +++ b/target/arm/tcg/mve_helper.c
> @@ -924,8 +924,8 @@ DO_1OP_IMM(vorri, DO_ORRI)
>          bool qc = false;                                                \
>          for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {              \
>              bool sat = false;                                           \
> -            TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);          \
> -            mergemask(&d[H##ESIZE(e)], r, mask);                        \
> +            TYPE r_ = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);         \
> +            mergemask(&d[H##ESIZE(e)], r_, mask);                       \
>              qc |= sat & mask & 1;                                       \
>          }                                                               \
>          if (qc) {                                                       \

The commit message doesn't list an error message relating to
this change and it's not immediately obvious to me what 'r'
would be shadowing here.

> @@ -2460,7 +2460,7 @@ static inline int64_t do_sqrshl48_d(int64_t src, int64_t shift,
>              return extval;
>          }
>      } else if (shift < 48) {
> -        int64_t extval = sextract64(src << shift, 0, 48);
> +        extval = sextract64(src << shift, 0, 48);
>          if (!sat || src == (extval >> shift)) {
>              return extval;
>          }
> @@ -2492,7 +2492,7 @@ static inline uint64_t do_uqrshl48_d(uint64_t src, int64_t shift,
>              return extval;
>          }
>      } else if (shift < 48) {
> -        uint64_t extval = extract64(src << shift, 0, 48);
> +        extval = extract64(src << shift, 0, 48);
>          if (!sat || src == (extval >> shift)) {
>              return extval;
>          }

These two parts are good, but one of them is missing from the
listed error messages in the commit message.

> diff --git a/target/arm/tcg/translate-m-nocp.c b/target/arm/tcg/translate-m-nocp.c
> index 33f6478bb9..42308c4db5 100644
> --- a/target/arm/tcg/translate-m-nocp.c
> +++ b/target/arm/tcg/translate-m-nocp.c
> @@ -506,7 +506,7 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
>
>          gen_branch_fpInactive(s, TCG_COND_EQ, lab_active);
>          /* fpInactive case: reads as FPDSCR_NS */
> -        TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
> +        tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
>          storefn(s, opaque, tmp, true);
>          lab_end = gen_new_label();
>          tcg_gen_br(lab_end);

This one's right too.

thanks
-- PMM

Re: [PATCH 05/11] hw/arm/virt: Clean up local variable shadowing

[Peter Maydell]

On Thu, 31 Aug 2023 at 23:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Fix:
>
>   hw/arm/virt.c:821:22: error: declaration shadows a local variable [-Werror,-Wshadow]
>             qemu_irq irq = qdev_get_gpio_in(vms->gic,
>                      ^
>   hw/arm/virt.c:803:13: note: previous declaration is here
>         int irq;
>             ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 06/11] hw/arm/allwinner: Clean up local variable shadowing

[Peter Maydell]

On Thu, 31 Aug 2023 at 23:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Fix:
>
>   hw/arm/allwinner-r40.c:412:14: error: declaration shadows a local variable [-Werror,-Wshadow]
>     for (int i = 0; i < AW_R40_NUM_MMCS; i++) {
>              ^
>   hw/arm/allwinner-r40.c:299:14: note: previous declaration is here
>     unsigned i;
>              ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 04/11] target/m68k: Clean up local variable shadowing

[Peter Maydell]

On Thu, 31 Aug 2023 at 23:58, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Fix:
>
>   target/m68k/translate.c:828:18: error: declaration shadows a local variable [-Werror,-Wshadow]
>             TCGv tmp = tcg_temp_new();
>                  ^
>   target/m68k/translate.c:801:15: note: previous declaration is here
>     TCGv reg, tmp, result;
>               ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/m68k/translate.c | 2 +-

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 01/11] tcg: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

On 1/9/23 02:38, Richard Henderson wrote:
> On 8/31/23 15:55, Philippe Mathieu-Daudé wrote:
>> -                    MemOp op = get_memop(oi);
>> +                    MemOp o = get_memop(oi);
> 
> mop would be a more descriptive replacement.

I went with that first, but then noticed 'MemOpIdx oi' and
renamed as 'o'. Anyway I'll change to 'mop', thanks!

> 
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~

Re: [PATCH 10/11] net/eth: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

On 1/9/23 09:07, Akihiko Odaki wrote:
> On 2023/09/01 7:56, Philippe Mathieu-Daudé wrote:
>> Fix:
>>
>>    net/eth.c:435:20: error: declaration shadows a local variable 
>> [-Werror,-Wshadow]
>>              size_t input_size = iov_size(pkt, pkt_frags);
>>                     ^
>>    net/eth.c:413:16: note: previous declaration is here
>>          size_t input_size = iov_size(pkt, pkt_frags);
>>                 ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   net/eth.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/eth.c b/net/eth.c
>> index 649e66bb1f..cf030eed7b 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -432,7 +432,7 @@ _eth_get_rss_ex_src_addr(const struct iovec *pkt, 
>> int pkt_frags,
>>           }
>>           if (opthdr.type == IP6_OPT_HOME) {
>> -            size_t input_size = iov_size(pkt, pkt_frags);
>> +            input_size = iov_size(pkt, pkt_frags);
> 
> You can just remove this statement.

Clever eh, thanks!

Re: [PATCH 02/11] target/arm: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

On 1/9/23 12:46, Peter Maydell wrote:
> On Thu, 31 Aug 2023 at 23:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Fix:
>>
>>    target/arm/tcg/translate-m-nocp.c:509:18: error: declaration shadows a local variable [-Werror,-Wshadow]
>>          TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
>>                   ^
>>    target/arm/tcg/translate-m-nocp.c:433:14: note: previous declaration is here
>>      TCGv_i32 tmp;
>>               ^
>>    target/arm/tcg/mve_helper.c:2463:17: error: declaration shadows a local variable [-Werror,-Wshadow]
>>          int64_t extval = sextract64(src << shift, 0, 48);
>>                  ^
>>    target/arm/tcg/mve_helper.c:2443:18: note: previous declaration is here
>>      int64_t val, extval;
>>                   ^
>>    target/arm/hvf/hvf.c:1936:13: error: declaration shadows a local variable [-Werror,-Wshadow]
>>          int ret = 0;
>>              ^
>>    target/arm/hvf/hvf.c:1807:9: note: previous declaration is here
>>      int ret;
>>          ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/arm/hvf/hvf.c              | 1 -
>>   target/arm/tcg/mve_helper.c       | 8 ++++----
>>   target/arm/tcg/translate-m-nocp.c | 2 +-
>>   3 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
>> index 486f90be1d..20d534faef 100644
>> --- a/target/arm/hvf/hvf.c
>> +++ b/target/arm/hvf/hvf.c
>> @@ -1933,7 +1933,6 @@ int hvf_vcpu_exec(CPUState *cpu)
>>           uint32_t rt = (syndrome >> 5) & 0x1f;
>>           uint32_t reg = syndrome & SYSREG_MASK;
>>           uint64_t val;
>> -        int ret = 0;
>>
>>           if (isread) {
>>               ret = hvf_sysreg_read(cpu, reg, rt);
> 
> I'm not sure this is correct.
> 
> The hvf_vcpu_exec() function is not documented, but in practice
> its caller expects it to return either EXCP_DEBUG (for "this was
> a guest debug exception you need to deal with") or something else
> (presumably the intention being 0 for OK).
> 
> The hvf_sysreg_read() and hvf_sysreg_write() functions are also not
> documented, but they return 0 on success, or 1 for a completely
> unrecognized sysreg where we've raised the UNDEF exception (but
> not if we raised an UNDEF exception for an unrecognized GIC sysreg --
> I think this is a bug). We use this return value to decide whether
> we need to advance the PC past the insn or not. It's not the same
> as the return value we want to return from hvf_vcpu_exec().

Indeed.

> So I think the correct fix here is to retain the variable as
> locally scoped but give it a name that doesn't clash with the
> other function-scoped variable.

OK.

>> diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c
>> index 403b345ea3..32087b6f0a 100644
>> --- a/target/arm/tcg/mve_helper.c
>> +++ b/target/arm/tcg/mve_helper.c
>> @@ -924,8 +924,8 @@ DO_1OP_IMM(vorri, DO_ORRI)
>>           bool qc = false;                                                \
>>           for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {              \
>>               bool sat = false;                                           \
>> -            TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);          \
>> -            mergemask(&d[H##ESIZE(e)], r, mask);                        \
>> +            TYPE r_ = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);         \
>> +            mergemask(&d[H##ESIZE(e)], r_, mask);                       \
>>               qc |= sat & mask & 1;                                       \
>>           }                                                               \
>>           if (qc) {                                                       \
> 
> The commit message doesn't list an error message relating to
> this change and it's not immediately obvious to me what 'r'
> would be shadowing here.

Full error:

target/arm/tcg/mve_helper.c: In function ‘helper_mve_vqshlsb’:
target/arm/tcg/mve_helper.c:1259:19: warning: declaration of ‘r’ shadows 
a previous local [-Wshadow=compatible-local]
  1259 |         typeof(N) r = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, 
&su32);  \
       |                   ^
target/arm/tcg/mve_helper.c:1267:5: note: in expansion of macro 
‘WRAP_QRSHL_HELPER’
  1267 |     WRAP_QRSHL_HELPER(do_sqrshl_bhs, N, M, false, satp)
       |     ^~~~~~~~~~~~~~~~~
target/arm/tcg/mve_helper.c:927:22: note: in expansion of macro 
‘DO_SQSHL_OP’
   927 |             TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat); 
         \
       |                      ^~
target/arm/tcg/mve_helper.c:945:5: note: in expansion of macro ‘DO_2OP_SAT’
   945 |     DO_2OP_SAT(OP##b, 1, int8_t, FN)            \
       |     ^~~~~~~~~~
target/arm/tcg/mve_helper.c:1277:1: note: in expansion of macro 
‘DO_2OP_SAT_S’
  1277 | DO_2OP_SAT_S(vqshls, DO_SQSHL_OP)
       | ^~~~~~~~~~~~

So 'r' comes from:

#define WRAP_QRSHL_HELPER(FN, N, M, ROUND, satp) \
     ({                                           \
         uint32_t su32 = 0;                       \
         typeof(N) r = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, &su32); \
         if (su32) {                              \
             *satp = true;                        \
         }                                        \
         r;                                       \
     })

I'll rename this one as 'qrshl_ret', and the previous one
'vqdmladh_ret'.

>> @@ -2460,7 +2460,7 @@ static inline int64_t do_sqrshl48_d(int64_t src, int64_t shift,
>>               return extval;
>>           }
>>       } else if (shift < 48) {
>> -        int64_t extval = sextract64(src << shift, 0, 48);
>> +        extval = sextract64(src << shift, 0, 48);
>>           if (!sat || src == (extval >> shift)) {
>>               return extval;
>>           }
>> @@ -2492,7 +2492,7 @@ static inline uint64_t do_uqrshl48_d(uint64_t src, int64_t shift,
>>               return extval;
>>           }
>>       } else if (shift < 48) {
>> -        uint64_t extval = extract64(src << shift, 0, 48);
>> +        extval = extract64(src << shift, 0, 48);
>>           if (!sat || src == (extval >> shift)) {
>>               return extval;
>>           }
> 
> These two parts are good, but one of them is missing from the
> listed error messages in the commit message.

I'll record both.

Thanks,

Phil.