Re: Help wanted for enabling -Wshadow=local

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> If you are in To:, we need your help to enable -Wshadow=local.
>>
>> 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.
>>
>> I collected the patches posted so far:
>>
>>     [PATCH v3 0/7] Steps towards enabling -Wshadow=local
>>     Message-ID: <20230921121312.1301864-1-armbru@redhat.com>
>>
>>     [PATCH v2 00/22] (few more) Steps towards enabling -Wshadow
>>     Message-ID: <20230904161235.84651-1-philmd@linaro.org>
>>
>>     [PATCH 0/3] (few more) Steps towards enabling -Wshadow [3 more]
>>     Message-ID: <20230904162824.85385-1-philmd@linaro.org>
>>
>>     [PATCH 0/8] ppc: Clean up local variable shadowing
>>     Message-ID: <20230918145850.241074-1-clg@kaod.org>
>>
>> Thanks, Philippe & Cedric!
>
> Collected since:
>
>       [PATCH] test-throttle: don't shadow 'index' variable in 
> do_test_accounting()
>       Message-Id: <20230922105742.81317-1-berto@igalia.com>
>
>       [PATCH] hw/acpi: changes towards enabling -Wshadow=local
>       Message-Id: <20230922124203.127110-1-anisinha@redhat.com>
>
>       [PATCH 0/4] arm: fix some -Wshadow warnings
>       Message-Id: <20230922152944.3583438-1-peter.maydell@linaro.org>
>
>       [PATCH 0/4] aspeed: Clean up local variable shadowing
>       Message-ID: <20230922155924.1172019-1-clg@kaod.org>
>
>       [PATCH] intel_iommu: Fix shadow local variables on "size"
>       Message-ID: <20230922160410.138786-1-peterx@redhat.com>
>
>       [PATCH 0/2] remove some variable shadowing
>       Message-ID: <20230922160644.438631-1-berrange@redhat.com>
>
>       [PATCH] tests/qtest/m48t59-test: Silence compiler warning with -Wshadow
>       Message-ID: <20230922163742.149444-1-thuth@redhat.com>
>
> Thanks Berto, Ani, Peter, Cédric, Peter, Daniel, and Thomas!

More:

        [PATCH] qemu-nbd: changes towards enabling -Wshadow=local
        Message-ID: <20230922205019.2755352-2-eblake@redhat.com>

        [PATCH] target/ppc: Rename variables to avoid local variable shadowing 
in VUPKPX
        Message-ID: <20230923071203.1209663-1-clg@kaod.org>

Thanks Eric and Cédric!

>> In my local build with -Wshadow=local, warnings remain in just 56 files.
>
> Down to 42.

Down to 40.

>> Sorted by subsystems, files covered by multiple subsystems marked
>> "(*NUMBER*)", files not covered in MAINTAINERS marked (*guess*):
>>
>> Guest CPU cores (TCG)
>> ---------------------
>> ARM TCG CPUs
>> M: Peter Maydell <peter.maydell@linaro.org>
>>     hw/arm/smmuv3-internal.h(*2*)
>>     hw/arm/smmuv3.c(*2*)
>
> Subsystem appears clean now.
>
>> ARM SMMU
>> M: Eric Auger <eric.auger@redhat.com>
>>     hw/arm/smmuv3-internal.h(*2*)
>>     hw/arm/smmuv3.c(*2*)
>
> Subsystem appears clean now.
>
>> Hexagon TCG CPUs
>> M: Brian Cain <bcain@quicinc.com>
>>     target/hexagon/gen_helper_funcs.py
>>     target/hexagon/mmvec/macros.h
>>     target/hexagon/op_helper.c
>>     target/hexagon/translate.c
>>
>> M68K TCG CPUs
>> M: Laurent Vivier <laurent@vivier.eu>
>>     disas/m68k.c
>>
>> PowerPC TCG CPUs
>> M: Nicholas Piggin <npiggin@gmail.com>
>> M: Daniel Henrique Barboza <danielhb413@gmail.com>
>> R: Cédric Le Goater <clg@kaod.org>
>>     target/ppc/int_helper.c

Subsystem appears clean now.

>> RISC-V TCG CPUs
>>     hw/riscv/opentitan.c(*2*)
>>     target/riscv/cpu.c
>>     target/riscv/vector_helper.c
>>
>> X86 TCG CPUs
>> M: Paolo Bonzini <pbonzini@redhat.com>
>> M: Richard Henderson <richard.henderson@linaro.org>
>> M: Eduardo Habkost <eduardo@habkost.net>
>>     hw/i386/acpi-build.c(*3*)
>
> File appears clean now.
>
>>     hw/i386/acpi-microvm.c(*2*)
>>     hw/i386/intel_iommu.c(*3*)
>
> File appears clean now.
>
>>     hw/i386/pc.c(*2*)
>>     hw/i386/x86.c(*2*)
>>     target/i386/cpu.c(*guess*)
>>     target/i386/tcg/seg_helper.c
>>     target/i386/tcg/sysemu/svm_helper.c
>>     target/i386/tcg/translate.c
>>
>> Guest CPU Cores (KVM)
>> ---------------------
>> X86 KVM CPUs
>> M: Paolo Bonzini <pbonzini@redhat.com>
>> M: Marcelo Tosatti <mtosatti@redhat.com>
>>     target/i386/kvm/kvm.c
>>
>> ARM Machines
>> ------------
>> ARM cores
>> M: Peter Maydell <peter.maydell@linaro.org>
>>     hw/intc/arm_gicv3_its.c
>
> Subsystem appears clean now.
>
>> Versatile PB
>> M: Peter Maydell <peter.maydell@linaro.org>
>>     hw/misc/arm_sysctl.c
>
> Subsystem appears clean now.
>
>> ASPEED BMCs
>> M: Cédric Le Goater <clg@kaod.org>
>> M: Peter Maydell <peter.maydell@linaro.org>
>> R: Andrew Jeffery <andrew@aj.id.au>
>> R: Joel Stanley <joel@jms.id.au>
>>     hw/i2c/aspeed_i2c.c
>>     hw/misc/aspeed_i3c.c
>
> Subsystem appears clean now.
>
>> LoongArch Machines
>> ------------------
>> Virt
>> M: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> M: Song Gao <gaosong@loongson.cn>
>>     hw/loongarch/virt.c
>>
>> RISC-V Machines
>> ---------------
>> OpenTitan
>> M: Alistair Francis <Alistair.Francis@wdc.com>
>>     hw/riscv/opentitan.c(*2*)
>>
>> X86 Machines
>> ------------
>> PC
>> M: Michael S. Tsirkin <mst@redhat.com>
>> M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>     hw/i386/acpi-build.c(*3*)
>
> File appears clean now.
>
>>     hw/i386/acpi-microvm.c(*2*)
>>     hw/i386/intel_iommu.c(*3*)
>
> File appears clean now.
>
>>     hw/i386/pc.c(*2*)
>>     hw/i386/x86.c(*2*)
>>
>> PC Chipset
>> M: Michael S. Tsirkin <mst@redhat.com>
>> M: Paolo Bonzini <pbonzini@redhat.com>
>>     hw/i2c/pm_smbus.c
>>
>> Devices
>> -------
>> ACPI/SMBIOS
>> M: Michael S. Tsirkin <mst@redhat.com>
>> M: Igor Mammedov <imammedo@redhat.com>
>> R: Ani Sinha <anisinha@redhat.com>
>>     hw/acpi/cpu_hotplug.c
>>     hw/i386/acpi-build.c(*3*)
>>     hw/smbios/smbios.c
>
> Subsystem appears clean now.
>
>> Network devices
>> M: Jason Wang <jasowang@redhat.com>
>>     hw/net/vhost_net.c(*2*)
>>
>> SCSI
>> M: Paolo Bonzini <pbonzini@redhat.com>
>> R: Fam Zheng <fam@euphon.net>
>>     hw/scsi/mptsas.c
>>
>> USB
>> M: Gerd Hoffmann <kraxel@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 <mst@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 <mst@redhat.com>
>>     hw/virtio/virtio-pci.c
>>     include/hw/virtio/virtio-gpu.h(*2*)
>>
>> nvme
>> M: Keith Busch <kbusch@kernel.org>
>> M: Klaus Jensen <its@irrelevant.dk>
>>     hw/nvme/ns.c
>>
>> virtio-gpu
>> M: Gerd Hoffmann <kraxel@redhat.com>
>>     include/hw/virtio/virtio-gpu.h(*2*)
>>
>> vhost-user-gpu
>> M: Marc-André Lureau <marcandre.lureau@redhat.com>
>> R: Gerd Hoffmann <kraxel@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 <kraxel@redhat.com>
>> M: Marc-André Lureau <marcandre.lureau@redhat.com>
>>     audio/audio.c
>>
>> Open Sound System (OSS) Audio backend
>> M: Gerd Hoffmann <kraxel@redhat.com>
>>     audio/ossaudio.c
>>
>> Compute Express Link
>> M: Jonathan Cameron <jonathan.cameron@huawei.com>
>> R: Fan Ni <fan.ni@samsung.com>
>>     include/hw/cxl/cxl_device.h
>>
>> Device Tree
>> M: Alistair Francis <alistair.francis@wdc.com>
>> R: David Gibson <david@gibson.dropbear.id.au>
>>     softmmu/device_tree.c
>>
>> Dump
>> M: Marc-André Lureau <marcandre.lureau@redhat.com>
>>     dump/dump.c
>>
>> Main loop
>> M: Paolo Bonzini <pbonzini@redhat.com>
>>     softmmu/vl.c
>>
>> qtest
>> M: Thomas Huth <thuth@redhat.com>
>> M: Laurent Vivier <lvivier@redhat.com>
>> R: Paolo Bonzini <pbonzini@redhat.com>
>>     tests/qtest/m48t59-test.c
>
> Subsystem appears clean now.
>
>> Seccomp
>> M: Daniel P. Berrange <berrange@redhat.com>
>>     softmmu/qemu-seccomp.c
>>
>> Cryptography
>> M: Daniel P. Berrange <berrange@redhat.com>
>>     crypto/tls-cipher-suites.c
>
> Subsystem appears clean now.
>
>> Throttling infrastructure
>> M: Alberto Garcia <berto@igalia.com>
>>     tests/unit/test-throttle.c
>
> Subsystem appears clean now.
>
>> VT-d Emulation
>> M: Michael S. Tsirkin <mst@redhat.com>
>> M: Peter Xu <peterx@redhat.com>
>> R: Jason Wang <jasowang@redhat.com>
>>     hw/i386/intel_iommu.c(*3*)
>
> Subsystem appears clean now.
>
>> Usermode Emulation
>> ------------------
>> Linux user
>> M: Laurent Vivier <laurent@vivier.eu>
>>     linux-user/flatload.c
>>     linux-user/mmap.c
>>     linux-user/syscall.c
>>
>> Block drivers
>> -------------
>> Network Block Device (NBD)
>> M: Eric Blake <eblake@redhat.com>
>> M: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>     qemu-nbd.c

Subsystem appears clean now.