* [PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing
2023-09-25 4:30 [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Alistair Francis
@ 2023-09-25 4:30 ` Alistair Francis
2023-09-25 9:08 ` Daniel Henrique Barboza
2023-09-25 4:30 ` [PATCH 2/4] target/riscv: cpu: " Alistair Francis
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2023-09-25 4:30 UTC (permalink / raw)
To: david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, alistair23,
dbarboza, Alistair Francis, liweiwei, Bin Meng
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand. Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".
This patch removes the local variable shadowing. Tested by adding:
--extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
To configure
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/riscv/opentitan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 6a2fcc4ade..436503f1ba 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -227,7 +227,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
IRQ_M_TIMER));
/* SPI-Hosts */
- for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
+ for (i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
dev = DEVICE(&(s->spi_host[i]));
if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi_host[i]), errp)) {
return;
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing
2023-09-25 4:30 ` [PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing Alistair Francis
@ 2023-09-25 9:08 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-09-25 9:08 UTC (permalink / raw)
To: Alistair Francis, david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, liweiwei,
Bin Meng
On 9/25/23 01:30, Alistair Francis wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> This patch removes the local variable shadowing. Tested by adding:
>
> --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
>
> To configure
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> hw/riscv/opentitan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 6a2fcc4ade..436503f1ba 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -227,7 +227,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
> IRQ_M_TIMER));
>
> /* SPI-Hosts */
> - for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
> + for (i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
> dev = DEVICE(&(s->spi_host[i]));
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi_host[i]), errp)) {
> return;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] target/riscv: cpu: Fixup local variables shadowing
2023-09-25 4:30 [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Alistair Francis
2023-09-25 4:30 ` [PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing Alistair Francis
@ 2023-09-25 4:30 ` Alistair Francis
2023-09-25 9:09 ` Daniel Henrique Barboza
2023-09-25 4:30 ` [PATCH 3/4] target/riscv: vector_helper: " Alistair Francis
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2023-09-25 4:30 UTC (permalink / raw)
To: david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, alistair23,
dbarboza, Alistair Francis, liweiwei, Bin Meng
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand. Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".
This patch removes the local variable shadowing. Tested by adding:
--extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
To configure
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eeeb08a35a..4dd1daada0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -699,7 +699,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
CSR_MPMMASK,
};
- for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
+ for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
int csrno = dump_csrs[i];
target_ulong val = 0;
RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
@@ -742,7 +742,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
CSR_VTYPE,
CSR_VLENB,
};
- for (int i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
+ for (i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
int csrno = dump_rvv_csrs[i];
target_ulong val = 0;
RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] target/riscv: cpu: Fixup local variables shadowing
2023-09-25 4:30 ` [PATCH 2/4] target/riscv: cpu: " Alistair Francis
@ 2023-09-25 9:09 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-09-25 9:09 UTC (permalink / raw)
To: Alistair Francis, david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, liweiwei,
Bin Meng
On 9/25/23 01:30, Alistair Francis wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> This patch removes the local variable shadowing. Tested by adding:
>
> --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
>
> To configure
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> target/riscv/cpu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index eeeb08a35a..4dd1daada0 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -699,7 +699,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> CSR_MPMMASK,
> };
>
> - for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
> + for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
> int csrno = dump_csrs[i];
> target_ulong val = 0;
> RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
> @@ -742,7 +742,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> CSR_VTYPE,
> CSR_VLENB,
> };
> - for (int i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
> + for (i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
> int csrno = dump_rvv_csrs[i];
> target_ulong val = 0;
> RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] target/riscv: vector_helper: Fixup local variables shadowing
2023-09-25 4:30 [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Alistair Francis
2023-09-25 4:30 ` [PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing Alistair Francis
2023-09-25 4:30 ` [PATCH 2/4] target/riscv: cpu: " Alistair Francis
@ 2023-09-25 4:30 ` Alistair Francis
2023-09-25 9:09 ` Daniel Henrique Barboza
2023-09-25 4:30 ` [PATCH 4/4] softmmu/device_tree: " Alistair Francis
2023-09-25 9:11 ` [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Daniel Henrique Barboza
4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2023-09-25 4:30 UTC (permalink / raw)
To: david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, alistair23,
dbarboza, Alistair Francis, liweiwei, Bin Meng
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand. Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".
This patch removes the local variable shadowing. Tested by adding:
--extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
To configure
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/vector_helper.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 3fb05cc3d6..cba02c1320 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -516,7 +516,7 @@ ProbeSuccess:
k++;
continue;
}
- target_ulong addr = base + ((i * nf + k) << log2_esz);
+ addr = base + ((i * nf + k) << log2_esz);
ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
k++;
}
@@ -4791,9 +4791,10 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
uint32_t vta = vext_vta(desc); \
uint32_t vma = vext_vma(desc); \
- target_ulong i_max, i; \
+ target_ulong i_max, i_min, i; \
\
- i_max = MAX(MIN(s1 < vlmax ? vlmax - s1 : 0, vl), env->vstart); \
+ i_min = MIN(s1 < vlmax ? vlmax - s1 : 0, vl); \
+ i_max = MAX(i_min, env->vstart); \
for (i = env->vstart; i < i_max; ++i) { \
if (!vm && !vext_elem_mask(v0, i)) { \
/* set masked-off elements to 1s */ \
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] target/riscv: vector_helper: Fixup local variables shadowing
2023-09-25 4:30 ` [PATCH 3/4] target/riscv: vector_helper: " Alistair Francis
@ 2023-09-25 9:09 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-09-25 9:09 UTC (permalink / raw)
To: Alistair Francis, david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, liweiwei,
Bin Meng
On 9/25/23 01:30, Alistair Francis wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> This patch removes the local variable shadowing. Tested by adding:
>
> --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
>
> To configure
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> target/riscv/vector_helper.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 3fb05cc3d6..cba02c1320 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -516,7 +516,7 @@ ProbeSuccess:
> k++;
> continue;
> }
> - target_ulong addr = base + ((i * nf + k) << log2_esz);
> + addr = base + ((i * nf + k) << log2_esz);
> ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
> k++;
> }
> @@ -4791,9 +4791,10 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
> uint32_t vta = vext_vta(desc); \
> uint32_t vma = vext_vma(desc); \
> - target_ulong i_max, i; \
> + target_ulong i_max, i_min, i; \
> \
> - i_max = MAX(MIN(s1 < vlmax ? vlmax - s1 : 0, vl), env->vstart); \
> + i_min = MIN(s1 < vlmax ? vlmax - s1 : 0, vl); \
> + i_max = MAX(i_min, env->vstart); \
> for (i = env->vstart; i < i_max; ++i) { \
> if (!vm && !vext_elem_mask(v0, i)) { \
> /* set masked-off elements to 1s */ \
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] softmmu/device_tree: Fixup local variables shadowing
2023-09-25 4:30 [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Alistair Francis
` (2 preceding siblings ...)
2023-09-25 4:30 ` [PATCH 3/4] target/riscv: vector_helper: " Alistair Francis
@ 2023-09-25 4:30 ` Alistair Francis
2023-09-25 9:10 ` Daniel Henrique Barboza
2023-09-25 9:11 ` [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Daniel Henrique Barboza
4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2023-09-25 4:30 UTC (permalink / raw)
To: david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, alistair23,
dbarboza, Alistair Francis, liweiwei, Bin Meng
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand. Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".
This patch removes the local variable shadowing. Tested by adding:
--extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
To configure
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
softmmu/device_tree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 30aa3aea9f..eb5166ca36 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -418,9 +418,9 @@ int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
}
p = str = g_malloc0(total_len);
for (i = 0; i < len; i++) {
- int len = strlen(array[i]) + 1;
- pstrcpy(p, len, array[i]);
- p += len;
+ int offset = strlen(array[i]) + 1;
+ pstrcpy(p, offset, array[i]);
+ p += offset;
}
ret = qemu_fdt_setprop(fdt, node_path, prop, str, total_len);
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] softmmu/device_tree: Fixup local variables shadowing
2023-09-25 4:30 ` [PATCH 4/4] softmmu/device_tree: " Alistair Francis
@ 2023-09-25 9:10 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-09-25 9:10 UTC (permalink / raw)
To: Alistair Francis, david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, liweiwei,
Bin Meng
On 9/25/23 01:30, Alistair Francis wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> This patch removes the local variable shadowing. Tested by adding:
>
> --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
>
> To configure
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> softmmu/device_tree.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 30aa3aea9f..eb5166ca36 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -418,9 +418,9 @@ int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
> }
> p = str = g_malloc0(total_len);
> for (i = 0; i < len; i++) {
> - int len = strlen(array[i]) + 1;
> - pstrcpy(p, len, array[i]);
> - p += len;
> + int offset = strlen(array[i]) + 1;
> + pstrcpy(p, offset, array[i]);
> + p += offset;
> }
>
> ret = qemu_fdt_setprop(fdt, node_path, prop, str, total_len);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local
2023-09-25 4:30 [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Alistair Francis
` (3 preceding siblings ...)
2023-09-25 4:30 ` [PATCH 4/4] softmmu/device_tree: " Alistair Francis
@ 2023-09-25 9:11 ` Daniel Henrique Barboza
2023-09-29 6:56 ` Markus Armbruster
4 siblings, 1 reply; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-09-25 9:11 UTC (permalink / raw)
To: Alistair Francis, david, qemu-riscv, qemu-devel
Cc: bmeng.cn, alistair.francis, Liu Zhiwei, palmer, liweiwei,
Bin Meng, Markus Armbruster
CCing Markus since he might want to add these in his shadow-next tree.
Daniel
On 9/25/23 01:30, Alistair Francis wrote:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> This patch removes the local variable shadowing. Tested by adding:
>
> --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
>
> To configure
>
> Alistair Francis (4):
> hw/riscv: opentitan: Fixup local variables shadowing
> target/riscv: cpu: Fixup local variables shadowing
> target/riscv: vector_helper: Fixup local variables shadowing
> softmmu/device_tree: Fixup local variables shadowing
>
> hw/riscv/opentitan.c | 2 +-
> softmmu/device_tree.c | 6 +++---
> target/riscv/cpu.c | 4 ++--
> target/riscv/vector_helper.c | 7 ++++---
> 4 files changed, 10 insertions(+), 9 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local
2023-09-25 9:11 ` [PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local Daniel Henrique Barboza
@ 2023-09-29 6:56 ` Markus Armbruster
0 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2023-09-29 6:56 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Alistair Francis, david, qemu-riscv, qemu-devel, bmeng.cn,
alistair.francis, Liu Zhiwei, palmer, liweiwei, Bin Meng
Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:
> CCing Markus since he might want to add these in his shadow-next tree.
Queued, thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread