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