On Tue, Sep 26, 2023 at 8:43 AM Markus Armbruster <arm...@redhat.com> wrote:

> Brian, Gerd, Jason, Marc-André, Michael, we need your help to enable
> -Wshadow=local.
>
> Paolo, you already took care of several subsystems (thanks!), except you
> left a few warnings in target/i386/tcg/seg_helper.c.
>
>
> 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".
>
> Enabling -Wshadow would prevent bugs like this one.  But we have to
> clean up all the offenders first.
>
> People responded quickly to my first call for help.  Thank you so much!
>
> I'm collecting patches in my git repo at
> https://repo.or.cz/qemu/armbru.git in branch shadow-next, output of
> git-shortlog appended.  I'm happy to do pull requests.  I don't mind
> maintainers merging patches for their subsystems; interference should be
> minimal.
>
> My test build is down to 19 files with warnings.  Sorted by subsystems,
> files covered by multiple subsystems marked "(*NUMBER*)":
>

Please make sure it's disabled for the bsd-user build. I have 3 patches in
my queue
to fix that, but I'm recovering from an illness and trying to land a large
number of patches
from my gsoc student Karim and git publish is being a pain. If this can
wait a week, I'll
likely be better enough by then and can submit the patches. They are all
trivial, but one
depends on the tcg header cleanups.

Thanks

Warner


> Guest CPU cores (TCG)
> ---------------------
> Hexagon TCG CPUs
> M: Brian Cain <bc...@quicinc.com>
>     target/hexagon/gen_helper_funcs.py
>     target/hexagon/mmvec/macros.h
>     target/hexagon/op_helper.c
>     target/hexagon/translate.c
>
> X86 TCG CPUs
> M: Paolo Bonzini <pbonz...@redhat.com>
> M: Richard Henderson <richard.hender...@linaro.org>
> M: Eduardo Habkost <edua...@habkost.net>
>     target/i386/tcg/seg_helper.c
>
> Devices
> -------
> Network devices
> M: Jason Wang <jasow...@redhat.com>
>     hw/net/vhost_net.c(*2*)
>
> USB
> M: Gerd Hoffmann <kra...@redhat.com>
>     hw/usb/desc.c
>     hw/usb/dev-hub.c
>     hw/usb/dev-storage.c
>     hw/usb/hcd-xhci.c
>     hw/usb/host-libusb.c
>
> vhost
> M: Michael S. Tsirkin <m...@redhat.com>
>     contrib/vhost-user-gpu/vhost-user-gpu.c(*2*)
>     contrib/vhost-user-gpu/vugpu.h(*2*)
>     hw/net/vhost_net.c(*2*)
>     hw/virtio/vhost.c
>
> virtio
> M: Michael S. Tsirkin <m...@redhat.com>
>     hw/virtio/virtio-pci.c
>     include/hw/virtio/virtio-gpu.h(*2*)
>
> virtio-gpu
> M: Gerd Hoffmann <kra...@redhat.com>
>     include/hw/virtio/virtio-gpu.h(*2*)
>
> vhost-user-gpu
> M: Marc-André Lureau <marcandre.lur...@redhat.com>
> R: Gerd Hoffmann <kra...@redhat.com>
>     contrib/vhost-user-gpu/vhost-user-gpu.c(*2*)
>     contrib/vhost-user-gpu/vugpu.h(*2*)
>
> Subsystems
> ----------
> Overall Audio backends
> M: Gerd Hoffmann <kra...@redhat.com>
> M: Marc-André Lureau <marcandre.lur...@redhat.com>
>     audio/audio.c
>
> Open Sound System (OSS) Audio backend
> M: Gerd Hoffmann <kra...@redhat.com>
>     audio/ossaudio.c
>
> Dump
> M: Marc-André Lureau <marcandre.lur...@redhat.com>
>     dump/dump.c
>
>
> Patches collected so far:
>
> Alberto Garcia (1):
>       test-throttle: don't shadow 'index' variable in do_test_accounting()
>
> 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
>
> Ani Sinha (1):
>       hw/acpi: changes towards enabling -Wshadow=local
>
> Cédric Le Goater (13):
>       hw/ppc: Clean up local variable shadowing in _FDT helper routine
>       pnv/psi: Clean up local variable shadowing
>       spapr: Clean up local variable shadowing in spapr_dt_cpus()
>       spapr: Clean up local variable shadowing in spapr_init_cpus()
>       spapr: Clean up local variable shadowing in spapr_get_fw_dev_path()
>       spapr/drc: Clean up local variable shadowing in
> rtas_ibm_configure_connector()
>       spapr/pci: Clean up local variable shadowing in spapr_phb_realize()
>       spapr/drc: Clean up local variable shadowing in prop_get_fdt()
>       aspeed/i2c: Clean up local variable shadowing
>       aspeed: Clean up local variable shadowing
>       aspeed/i3c: Rename variable shadowing a local
>       aspeed/timer: Clean up local variable shadowing
>       target/ppc: Rename variables to avoid local variable shadowing in
> VUPKPX
>
> Daniel P. Berrangé (2):
>       crypto: remove shadowed 'ret' variable
>       seccomp: avoid shadowing of 'action' variable
>
> Eric Blake (1):
>       qemu-nbd: changes towards enabling -Wshadow=local
>
> Klaus Jensen (1):
>       hw/nvme: Clean up local variable shadowing in nvme_ns_init()
>
> Laurent Vivier (1):
>       disas/m68k: clean up local variable shadowing
>
> Markus Armbruster (8):
>       meson: Enable -Wshadow as a warning
>       migration/rdma: Fix save_page method to fail on polling error
>       migration: Clean up local variable shadowing
>       ui: Clean up local variable shadowing
>       block/dirty-bitmap: Clean up local variable shadowing
>       block/vdi: Clean up local variable shadowing
>       block: Clean up local variable shadowing
>       qobject atomics osdep: Make a few macros more hygienic
>
> Paolo Bonzini (8):
>       mptsas: avoid shadowed local variables
>       pm_smbus: rename variable to avoid shadowing
>       vl: remove shadowed local variables
>       target/i386/kvm: eliminate shadowed local variables
>       target/i386/cpu: avoid shadowed local variables
>       target/i386/translate: avoid shadowed local variables
>       target/i386/svm_helper: eliminate duplicate local variable
>       target/i386/seg_helper: remove shadowed variable
>
> 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
>
> Peter Xu (1):
>       intel_iommu: Fix shadow local variables on "size"
>
> Philippe Mathieu-Daudé (23):
>       tcg: Clean up local variable shadowing
>       target/arm/tcg: Clean up local variable shadowing
>       target/arm/hvf: Clean up local variable shadowing
>       target/mips: Clean up local variable shadowing
>       target/m68k: Clean up local variable shadowing
>       target/tricore: Clean up local variable shadowing
>       hw/arm/armv7m: Clean up local variable shadowing
>       hw/arm/virt: Clean up local variable shadowing
>       hw/arm/allwinner: Clean up local variable shadowing
>       hw/ide/ahci: Clean up local variable shadowing
>       hw/m68k: Clean up local variable shadowing
>       hw/microblaze: Clean up local variable shadowing
>       hw/nios2: Clean up local variable shadowing
>       net/eth: Clean up local variable shadowing
>       crypto/cipher-gnutls.c: Clean up local variable shadowing
>       util/vhost-user-server: Clean up local variable shadowing
>       semihosting/arm-compat: Clean up local variable shadowing
>       linux-user/strace: Clean up local variable shadowing
>       sysemu/device_tree: Clean up local variable shadowing
>       softmmu/memory: Clean up local variable shadowing
>       softmmu/physmem: Clean up local variable shadowing
>       hw/core/machine: Clean up local variable shadowing
>       hw/intc/openpic: Clean up local variable shadowing
>
>
>

Reply via email to