[PATCH 0/4] arm: fix some -Wshadow warnings

[PATCH 0/4] arm: fix some -Wshadow warnings

[Peter Maydell]

These patches fix some -Wshadow warnings in arm related code.

-- PMM

Peter Maydell (4):
  hw/intc/arm_gicv3_its: Avoid shadowing variable in
    do_process_its_cmd()
  hw/misc/arm_sysctl.c: Avoid shadowing local variable
  hw/arm/smmuv3.c: Avoid shadowing variable
  hw/arm/smmuv3-internal.h: Don't use locals in statement macros

 hw/arm/smmuv3-internal.h | 41 +++++++++++++---------------------------
 hw/arm/smmuv3.c          |  4 ++--
 hw/intc/arm_gicv3_its.c  |  6 +++---
 hw/misc/arm_sysctl.c     |  6 +++---
 4 files changed, 21 insertions(+), 36 deletions(-)

-- 
2.34.1

[PATCH 1/4] hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()

[Peter Maydell]

Avoid shadowing a local variable in do_process_its_cmd():

../../hw/intc/arm_gicv3_its.c:548:17: warning: declaration of ‘ite’ shadows a previous local [-Wshadow=compatible-local]
  548 |         ITEntry ite = {};
      |                 ^~~
../../hw/intc/arm_gicv3_its.c:518:13: note: shadowed declaration is here
  518 |     ITEntry ite;
      |             ^~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gicv3_its.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 5f552b4d37f..52e9aca9c65 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -545,10 +545,10 @@ static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, uint32_t devid,
     }
 
     if (cmdres == CMD_CONTINUE_OK && cmd == DISCARD) {
-        ITEntry ite = {};
+        ITEntry i = {};
         /* remove mapping from interrupt translation table */
-        ite.valid = false;
-        return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : CMD_STALL;
+        i.valid = false;
+        return update_ite(s, eventid, &dte, &i) ? CMD_CONTINUE_OK : CMD_STALL;
     }
     return CMD_CONTINUE_OK;
 }
-- 
2.34.1

[PATCH 2/4] hw/misc/arm_sysctl.c: Avoid shadowing local variable

[Peter Maydell]

Avoid shadowing a local variable in arm_sysctl_write():

../../hw/misc/arm_sysctl.c: In function ‘arm_sysctl_write’:
../../hw/misc/arm_sysctl.c:537:26: warning: declaration of ‘val’ shadows a parameter [-Wshadow=local]
  537 |                 uint32_t val;
      |                          ^~~
../../hw/misc/arm_sysctl.c:388:39: note: shadowed declaration is here
  388 |                              uint64_t val, unsigned size)
      |                              ~~~~~~~~~^~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/arm_sysctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
index 42d46938543..3e4f4b05244 100644
--- a/hw/misc/arm_sysctl.c
+++ b/hw/misc/arm_sysctl.c
@@ -534,12 +534,12 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
                     s->sys_cfgstat |= 2;        /* error */
                 }
             } else {
-                uint32_t val;
+                uint32_t data;
                 if (!vexpress_cfgctrl_read(s, dcc, function, site, position,
-                                           device, &val)) {
+                                           device, &data)) {
                     s->sys_cfgstat |= 2;        /* error */
                 } else {
-                    s->sys_cfgdata = val;
+                    s->sys_cfgdata = data;
                 }
             }
         }
-- 
2.34.1

[PATCH 3/4] hw/arm/smmuv3.c: Avoid shadowing variable

[Peter Maydell]

Avoid shadowing a variable in smmuv3_notify_iova():

../../hw/arm/smmuv3.c: In function ‘smmuv3_notify_iova’:
../../hw/arm/smmuv3.c:1043:23: warning: declaration of ‘event’ shadows a previous local [-Wshadow=local]
 1043 |         SMMUEventInfo event = {.inval_ste_allowed = true};
      |                       ^~~~~
../../hw/arm/smmuv3.c:1038:19: note: shadowed declaration is here
 1038 |     IOMMUTLBEvent event;
      |                   ^~~~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/smmuv3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 1e9be8e89af..6f2b2bd45f9 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1040,8 +1040,8 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
     SMMUv3State *s = sdev->smmu;
 
     if (!tg) {
-        SMMUEventInfo event = {.inval_ste_allowed = true};
-        SMMUTransCfg *cfg = smmuv3_get_config(sdev, &event);
+        SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
+        SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
         SMMUTransTableInfo *tt;
 
         if (!cfg) {
-- 
2.34.1

[PATCH 4/4] hw/arm/smmuv3-internal.h: Don't use locals in statement macros

[Peter Maydell]

The STE_CTXPTR() and STE_S2TTB() macros both extract two halves
of an address from fields in the STE and combine them into a
single value to return. The current code for this uses a GCC
statement expression. There are two problems with this:

(1) The type chosen for the variable in the statement expr
is 'unsigned long', which might not be 64 bits

(2) the name chosen for the variable causes -Wshadow warnings
because it's the same as a variable in use at the callsite:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmu_get_cd’:
../../hw/arm/smmuv3-internal.h:538:23: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
  538 |         unsigned long addr;                                     \
      |                       ^~~~
../../hw/arm/smmuv3.c:339:23: note: in expansion of macro ‘STE_CTXPTR’
  339 |     dma_addr_t addr = STE_CTXPTR(ste);
      |                       ^~~~~~~~~~
../../hw/arm/smmuv3.c:339:16: note: shadowed declaration is here
  339 |     dma_addr_t addr = STE_CTXPTR(ste);
      |                ^~~~

Sidestep both of these problems by just using a single
expression rather than a statement expr.

For CMD_ADDR, we got the type of the variable right but still
run into -Wshadow problems:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmuv3_range_inval’:
../../hw/arm/smmuv3-internal.h:334:22: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
  334 |             uint64_t addr = high << 32 | (low << 12);         \
      |                      ^~~~
../../hw/arm/smmuv3.c:1104:28: note: in expansion of macro ‘CMD_ADDR’
 1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
      |                            ^~~~~~~~
../../hw/arm/smmuv3.c:1104:21: note: shadowed declaration is here
 1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
      |                     ^~~~

so convert it too.

CD_TTB has neither problem, but it is the only other macro in
the file that uses this pattern, so we convert it also for
consistency's sake.

We use extract64() rather than extract32() to avoid having
to explicitly cast the result to uint64_t.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/smmuv3-internal.h | 41 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 6d1c1edab7b..648c2e37a27 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -328,12 +328,9 @@ enum { /* Command completion notification */
 #define CMD_TTL(x)          extract32((x)->word[2], 8 , 2)
 #define CMD_TG(x)           extract32((x)->word[2], 10, 2)
 #define CMD_STE_RANGE(x)    extract32((x)->word[2], 0 , 5)
-#define CMD_ADDR(x) ({                                        \
-            uint64_t high = (uint64_t)(x)->word[3];           \
-            uint64_t low = extract32((x)->word[2], 12, 20);    \
-            uint64_t addr = high << 32 | (low << 12);         \
-            addr;                                             \
-        })
+#define CMD_ADDR(x)                             \
+    (((uint64_t)((x)->word[3]) << 32) |         \
+     ((extract64((x)->word[2], 12, 20)) << 12))
 
 #define SMMU_FEATURE_2LVL_STE (1 << 0)
 
@@ -533,21 +530,13 @@ typedef struct CD {
 #define STE_S2S(x)         extract32((x)->word[5], 25, 1)
 #define STE_S2R(x)         extract32((x)->word[5], 26, 1)
 
-#define STE_CTXPTR(x)                                           \
-    ({                                                          \
-        unsigned long addr;                                     \
-        addr = (uint64_t)extract32((x)->word[1], 0, 16) << 32;  \
-        addr |= (uint64_t)((x)->word[0] & 0xffffffc0);          \
-        addr;                                                   \
-    })
+#define STE_CTXPTR(x)                                   \
+    ((extract64((x)->word[1], 0, 16) << 32) |           \
+     ((x)->word[0] & 0xffffffc0))
 
-#define STE_S2TTB(x)                                            \
-    ({                                                          \
-        unsigned long addr;                                     \
-        addr = (uint64_t)extract32((x)->word[7], 0, 16) << 32;  \
-        addr |= (uint64_t)((x)->word[6] & 0xfffffff0);          \
-        addr;                                                   \
-    })
+#define STE_S2TTB(x)                                    \
+    ((extract64((x)->word[7], 0, 16) << 32) |           \
+     ((x)->word[6] & 0xfffffff0))
 
 static inline int oas2bits(int oas_field)
 {
@@ -585,14 +574,10 @@ static inline int pa_range(STE *ste)
 
 #define CD_VALID(x)   extract32((x)->word[0], 31, 1)
 #define CD_ASID(x)    extract32((x)->word[1], 16, 16)
-#define CD_TTB(x, sel)                                      \
-    ({                                                      \
-        uint64_t hi, lo;                                    \
-        hi = extract32((x)->word[(sel) * 2 + 3], 0, 19);    \
-        hi <<= 32;                                          \
-        lo = (x)->word[(sel) * 2 + 2] & ~0xfULL;            \
-        hi | lo;                                            \
-    })
+#define CD_TTB(x, sel)                                          \
+    ((extract64((x)->word[(sel) * 2 + 3], 0, 19) << 32) |       \
+     ((x)->word[(sel) * 2 + 2] & ~0xfULL))
+
 #define CD_HAD(x, sel)   extract32((x)->word[(sel) * 2 + 2], 1, 1)
 
 #define CD_TSZ(x, sel)   extract32((x)->word[0], (16 * (sel)) + 0, 6)
-- 
2.34.1

Re: [PATCH 1/4] hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()

[Philippe Mathieu-Daudé]

On 22/9/23 17:29, Peter Maydell wrote:
> Avoid shadowing a local variable in do_process_its_cmd():
> 
> ../../hw/intc/arm_gicv3_its.c:548:17: warning: declaration of ‘ite’ shadows a previous local [-Wshadow=compatible-local]
>    548 |         ITEntry ite = {};
>        |                 ^~~
> ../../hw/intc/arm_gicv3_its.c:518:13: note: shadowed declaration is here
>    518 |     ITEntry ite;
>        |             ^~~
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_its.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

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

Re: [PATCH 2/4] hw/misc/arm_sysctl.c: Avoid shadowing local variable

[Philippe Mathieu-Daudé]

On 22/9/23 17:29, Peter Maydell wrote:
> Avoid shadowing a local variable in arm_sysctl_write():
> 
> ../../hw/misc/arm_sysctl.c: In function ‘arm_sysctl_write’:
> ../../hw/misc/arm_sysctl.c:537:26: warning: declaration of ‘val’ shadows a parameter [-Wshadow=local]
>    537 |                 uint32_t val;
>        |                          ^~~
> ../../hw/misc/arm_sysctl.c:388:39: note: shadowed declaration is here
>    388 |                              uint64_t val, unsigned size)
>        |                              ~~~~~~~~~^~~
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/arm_sysctl.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

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

Re: [PATCH 3/4] hw/arm/smmuv3.c: Avoid shadowing variable

[Philippe Mathieu-Daudé]

On 22/9/23 17:29, Peter Maydell wrote:
> Avoid shadowing a variable in smmuv3_notify_iova():
> 
> ../../hw/arm/smmuv3.c: In function ‘smmuv3_notify_iova’:
> ../../hw/arm/smmuv3.c:1043:23: warning: declaration of ‘event’ shadows a previous local [-Wshadow=local]
>   1043 |         SMMUEventInfo event = {.inval_ste_allowed = true};
>        |                       ^~~~~
> ../../hw/arm/smmuv3.c:1038:19: note: shadowed declaration is here
>   1038 |     IOMMUTLBEvent event;
>        |                   ^~~~~
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/smmuv3.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

Re: [PATCH 4/4] hw/arm/smmuv3-internal.h: Don't use locals in statement macros

[Eric Auger]

Hi Peter,

On 9/22/23 17:29, Peter Maydell wrote:
> The STE_CTXPTR() and STE_S2TTB() macros both extract two halves
> of an address from fields in the STE and combine them into a
> single value to return. The current code for this uses a GCC
> statement expression. There are two problems with this:
>
> (1) The type chosen for the variable in the statement expr
> is 'unsigned long', which might not be 64 bits
>
> (2) the name chosen for the variable causes -Wshadow warnings
> because it's the same as a variable in use at the callsite:
>
> In file included from ../../hw/arm/smmuv3.c:34:
> ../../hw/arm/smmuv3.c: In function ‘smmu_get_cd’:
> ../../hw/arm/smmuv3-internal.h:538:23: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
>   538 |         unsigned long addr;                                     \
>       |                       ^~~~
> ../../hw/arm/smmuv3.c:339:23: note: in expansion of macro ‘STE_CTXPTR’
>   339 |     dma_addr_t addr = STE_CTXPTR(ste);
>       |                       ^~~~~~~~~~
> ../../hw/arm/smmuv3.c:339:16: note: shadowed declaration is here
>   339 |     dma_addr_t addr = STE_CTXPTR(ste);
>       |                ^~~~
>
> Sidestep both of these problems by just using a single
> expression rather than a statement expr.
>
> For CMD_ADDR, we got the type of the variable right but still
> run into -Wshadow problems:
>
> In file included from ../../hw/arm/smmuv3.c:34:
> ../../hw/arm/smmuv3.c: In function ‘smmuv3_range_inval’:
> ../../hw/arm/smmuv3-internal.h:334:22: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
>   334 |             uint64_t addr = high << 32 | (low << 12);         \
>       |                      ^~~~
> ../../hw/arm/smmuv3.c:1104:28: note: in expansion of macro ‘CMD_ADDR’
>  1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
>       |                            ^~~~~~~~
> ../../hw/arm/smmuv3.c:1104:21: note: shadowed declaration is here
>  1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
>       |                     ^~~~
>
> so convert it too.
>
> CD_TTB has neither problem, but it is the only other macro in
> the file that uses this pattern, so we convert it also for
> consistency's sake.
>
> We use extract64() rather than extract32() to avoid having
> to explicitly cast the result to uint64_t.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/smmuv3-internal.h | 41 +++++++++++++---------------------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 6d1c1edab7b..648c2e37a27 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -328,12 +328,9 @@ enum { /* Command completion notification */
>  #define CMD_TTL(x)          extract32((x)->word[2], 8 , 2)
>  #define CMD_TG(x)           extract32((x)->word[2], 10, 2)
>  #define CMD_STE_RANGE(x)    extract32((x)->word[2], 0 , 5)
> -#define CMD_ADDR(x) ({                                        \
> -            uint64_t high = (uint64_t)(x)->word[3];           \
> -            uint64_t low = extract32((x)->word[2], 12, 20);    \
> -            uint64_t addr = high << 32 | (low << 12);         \
> -            addr;                                             \
> -        })
> +#define CMD_ADDR(x)                             \
> +    (((uint64_t)((x)->word[3]) << 32) |         \
> +     ((extract64((x)->word[2], 12, 20)) << 12))
>  
>  #define SMMU_FEATURE_2LVL_STE (1 << 0)
>  
> @@ -533,21 +530,13 @@ typedef struct CD {
>  #define STE_S2S(x)         extract32((x)->word[5], 25, 1)
>  #define STE_S2R(x)         extract32((x)->word[5], 26, 1)
>  
> -#define STE_CTXPTR(x)                                           \
> -    ({                                                          \
> -        unsigned long addr;                                     \
> -        addr = (uint64_t)extract32((x)->word[1], 0, 16) << 32;  \
> -        addr |= (uint64_t)((x)->word[0] & 0xffffffc0);          \
> -        addr;                                                   \
> -    })
> +#define STE_CTXPTR(x)                                   \
> +    ((extract64((x)->word[1], 0, 16) << 32) |           \
> +     ((x)->word[0] & 0xffffffc0))
>  
> -#define STE_S2TTB(x)                                            \
> -    ({                                                          \
> -        unsigned long addr;                                     \
> -        addr = (uint64_t)extract32((x)->word[7], 0, 16) << 32;  \
> -        addr |= (uint64_t)((x)->word[6] & 0xfffffff0);          \
> -        addr;                                                   \
> -    })
> +#define STE_S2TTB(x)                                    \
> +    ((extract64((x)->word[7], 0, 16) << 32) |           \
> +     ((x)->word[6] & 0xfffffff0))
>  
>  static inline int oas2bits(int oas_field)
>  {
> @@ -585,14 +574,10 @@ static inline int pa_range(STE *ste)
>  
>  #define CD_VALID(x)   extract32((x)->word[0], 31, 1)
>  #define CD_ASID(x)    extract32((x)->word[1], 16, 16)
> -#define CD_TTB(x, sel)                                      \
> -    ({                                                      \
> -        uint64_t hi, lo;                                    \
> -        hi = extract32((x)->word[(sel) * 2 + 3], 0, 19);    \
> -        hi <<= 32;                                          \
> -        lo = (x)->word[(sel) * 2 + 2] & ~0xfULL;            \
> -        hi | lo;                                            \
> -    })
> +#define CD_TTB(x, sel)                                          \
> +    ((extract64((x)->word[(sel) * 2 + 3], 0, 19) << 32) |       \
> +     ((x)->word[(sel) * 2 + 2] & ~0xfULL))
> +
>  #define CD_HAD(x, sel)   extract32((x)->word[(sel) * 2 + 2], 1, 1)
>  
>  #define CD_TSZ(x, sel)   extract32((x)->word[0], (16 * (sel)) + 0, 6)
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

Re: [PATCH 3/4] hw/arm/smmuv3.c: Avoid shadowing variable

[Eric Auger]

Hi Peter,

On 9/22/23 17:29, Peter Maydell wrote:
> Avoid shadowing a variable in smmuv3_notify_iova():
>
> ../../hw/arm/smmuv3.c: In function ‘smmuv3_notify_iova’:
> ../../hw/arm/smmuv3.c:1043:23: warning: declaration of ‘event’ shadows a previous local [-Wshadow=local]
>  1043 |         SMMUEventInfo event = {.inval_ste_allowed = true};
>       |                       ^~~~~
> ../../hw/arm/smmuv3.c:1038:19: note: shadowed declaration is here
>  1038 |     IOMMUTLBEvent event;
>       |                   ^~~~~
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/smmuv3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 1e9be8e89af..6f2b2bd45f9 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1040,8 +1040,8 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
>      SMMUv3State *s = sdev->smmu;
>  
>      if (!tg) {
> -        SMMUEventInfo event = {.inval_ste_allowed = true};
> -        SMMUTransCfg *cfg = smmuv3_get_config(sdev, &event);
> +        SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
> +        SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
>          SMMUTransTableInfo *tt;
>  
>          if (!cfg) {
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric

Re: [PATCH 2/4] hw/misc/arm_sysctl.c: Avoid shadowing local variable

[Eric Auger]

On 9/22/23 17:29, Peter Maydell wrote:
> Avoid shadowing a local variable in arm_sysctl_write():
>
> ../../hw/misc/arm_sysctl.c: In function ‘arm_sysctl_write’:
> ../../hw/misc/arm_sysctl.c:537:26: warning: declaration of ‘val’ shadows a parameter [-Wshadow=local]
>   537 |                 uint32_t val;
>       |                          ^~~
> ../../hw/misc/arm_sysctl.c:388:39: note: shadowed declaration is here
>   388 |                              uint64_t val, unsigned size)
>       |                              ~~~~~~~~~^~~
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/misc/arm_sysctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
> index 42d46938543..3e4f4b05244 100644
> --- a/hw/misc/arm_sysctl.c
> +++ b/hw/misc/arm_sysctl.c
> @@ -534,12 +534,12 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
>                      s->sys_cfgstat |= 2;        /* error */
>                  }
>              } else {
> -                uint32_t val;
> +                uint32_t data;
>                  if (!vexpress_cfgctrl_read(s, dcc, function, site, position,
> -                                           device, &val)) {
> +                                           device, &data)) {
>                      s->sys_cfgstat |= 2;        /* error */
>                  } else {
> -                    s->sys_cfgdata = val;
> +                    s->sys_cfgdata = data;
>                  }
>              }
>          }
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

Re: [PATCH 1/4] hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()

[Eric Auger]

On 9/22/23 17:29, Peter Maydell wrote:
> Avoid shadowing a local variable in do_process_its_cmd():
>
> ../../hw/intc/arm_gicv3_its.c:548:17: warning: declaration of ‘ite’ shadows a previous local [-Wshadow=compatible-local]
>   548 |         ITEntry ite = {};
>       |                 ^~~
> ../../hw/intc/arm_gicv3_its.c:518:13: note: shadowed declaration is here
>   518 |     ITEntry ite;
>       |             ^~~
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/arm_gicv3_its.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 5f552b4d37f..52e9aca9c65 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -545,10 +545,10 @@ static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, uint32_t devid,
>      }
>  
>      if (cmdres == CMD_CONTINUE_OK && cmd == DISCARD) {
> -        ITEntry ite = {};
> +        ITEntry i = {};
>          /* remove mapping from interrupt translation table */
> -        ite.valid = false;
> -        return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : CMD_STALL;
> +        i.valid = false;
> +        return update_ite(s, eventid, &dte, &i) ? CMD_CONTINUE_OK : CMD_STALL;
>      }
>      return CMD_CONTINUE_OK;
>  }
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

Re: [PATCH 0/4] arm: fix some -Wshadow warnings

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> These patches fix some -Wshadow warnings in arm related code.

Queued, thanks!