Re: [PATCH 0/7] Steps towards enabling -Wshadow=local

[Markus Armbruster <armbru@xxxxxxxxxx>]

Markus Armbruster <armbru@xxxxxxxxxx> writes:

> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand.  Bugs love to hide in such code.
> Evidence: PATCH 1.
>
> Enabling -Wshadow would prevent bugs like this one.  But we'd have to
> clean up all the offenders first.  We got a lot of them.
>
> Enabling -Wshadow=local should be less work for almost as much gain.
> I took a stab at it.  There's a small, exciting part, and a large,
> boring part.
>
> The exciting part is dark preprocessor sorcery to let us nest macro
> calls without shadowing: PATCH 7.
>
> The boring part is cleaning up all the other warnings.  I did some
> [PATCH 2-6], but ran out of steam long before finishing the job.  Some
> 160 unique warnings remain.
>
> To see them, enable -Wshadow=local like so:
>
> diff --git a/meson.build b/meson.build
> index 98e68ef0b1..9fc4c7ac9d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -466,6 +466,9 @@ warn_flags = [
>    '-Wno-tautological-type-limit-compare',
>    '-Wno-psabi',
>    '-Wno-gnu-variable-sized-type-not-at-end',
> +  '-Wshadow=local',
> +  '-Wno-error=shadow=local',
> +  '-Wno-error=shadow=compatible-local',
>  ]
>  
>  if targetos != 'darwin'
>
> You may want to drop the -Wno-error lines.
>
> Subsystems with -Wshadow=local warnings:
>
>     virtio-gpu
>     virtio
>     Device Tree
>     Overall TCG CPUs

Philippe's "[PATCH 00/11] (few more) Steps towards enabling -Wshadow"
takes care of this one.

>     Overall Audio backends
>     Open Sound System (OSS) Audio backend
>     vhost
>     vhost-user-gpu
>     Cryptography
>     M68K TCG CPUs
>     Dump
>     ACPI/SMBIOS
>     Allwinner-a10

Likewise.

>     ARM TCG CPUs
>     MPS2
>     ASPEED BMCs
>     ARM SMMU
>     Virt
>     Machine core
>     PC Chipset
>     X86 TCG CPUs
>     PC
>     VT-d Emulation
>     IDE

Likewise.

>     ARM cores
>     OpenPIC interrupt controller
>     q800

Likewise.

>     petalogix_ml605
>     MicroBlaze TCG CPUs
>     Versatile PB
>     Network devices
>     NiosII TCG CPUs
>     nvme
>     PowerNV (Non-Virtualized)
>     sPAPR (pseries)
>     OpenTitan
>     RISC-V TCG CPUs
>     SCSI
>     USB
>     Linux user
>     Network packet abstractions

Likewise.

>     Network device backends

Likewise.

>     Network Block Device (NBD)
>     Semihosting
>     Memory API
>     Seccomp
>     Main loop
>     Hexagon TCG CPUs
>     X86 KVM CPUs
>     MIPS TCG CPUs

Likewise.

>     PowerPC TCG CPUs
>     TriCore TCG CPUs
>     Common TCG code

Likewise.

>     qtest
>     Throttling infrastructure
>     Vhost-user block device backend server
>
> Files with -Wshadow=local warnings:
>
>     accel/tcg/tb-maint.c

Likewise.

>     audio/audio.c
>     audio/ossaudio.c
>     contrib/vhost-user-gpu/vhost-user-gpu.c
>     contrib/vhost-user-gpu/vugpu.h
>     crypto/cipher-gnutls.c.inc
>     crypto/tls-cipher-suites.c
>     disas/m68k.c
>     dump/dump.c
>     hw/acpi/cpu_hotplug.c
>     hw/arm/allwinner-r40.c

Likewise.

>     hw/arm/armsse.c
>     hw/arm/armv7m.c
>     hw/arm/aspeed_ast2600.c

Likewise.

>     hw/arm/smmuv3-internal.h
>     hw/arm/smmuv3.c
>     hw/arm/virt.c

Likewise.

>     hw/core/machine.c
>     hw/i2c/aspeed_i2c.c
>     hw/i2c/pm_smbus.c
>     hw/i386/acpi-build.c
>     hw/i386/acpi-microvm.c
>     hw/i386/intel_iommu.c
>     hw/i386/pc.c
>     hw/i386/x86.c
>     hw/ide/ahci.c

Likewise.

>     hw/intc/arm_gicv3_its.c
>     hw/intc/openpic.c
>     hw/loongarch/virt.c
>     hw/m68k/bootinfo.h

Likewise.

>     hw/microblaze/petalogix_ml605_mmu.c
>     hw/misc/arm_sysctl.c
>     hw/misc/aspeed_i3c.c
>     hw/net/vhost_net.c
>     hw/nios2/10m50_devboard.c
>     hw/nvme/ns.c
>     hw/ppc/pnv_psi.c
>     hw/ppc/spapr.c
>     hw/ppc/spapr_drc.c
>     hw/ppc/spapr_pci.c
>     hw/riscv/opentitan.c
>     hw/scsi/mptsas.c
>     hw/smbios/smbios.c
>     hw/usb/desc.c
>     hw/usb/dev-hub.c
>     hw/usb/dev-storage.c
>     hw/usb/hcd-xhci.c
>     hw/usb/host-libusb.c
>     hw/virtio/vhost.c
>     hw/virtio/virtio-pci.c
>     include/hw/cxl/cxl_device.h
>     include/hw/ppc/fdt.h
>     include/hw/virtio/virtio-gpu.h
>     include/sysemu/device_tree.h

Likewise.

>     linux-user/flatload.c
>     linux-user/mmap.c
>     linux-user/strace.c
>     linux-user/syscall.c
>     net/eth.c

Likewise.

>     qemu-nbd.c
>     semihosting/arm-compat-semi.c
>     softmmu/device_tree.c
>     softmmu/memory.c
>     softmmu/physmem.c
>     softmmu/qemu-seccomp.c
>     softmmu/vl.c

      target/arm/hvf/hvf.c

Likewise.

>     target/arm/tcg/mve_helper.c

Likewise.

>     target/arm/tcg/translate-m-nocp.c

Likewise.

>     target/hexagon/helper_funcs_generated.c.inc

This is actually

      target/hexagon/gen_helper_funcs.py

>     target/hexagon/mmvec/macros.h
>     target/hexagon/op_helper.c
>     target/hexagon/translate.c
>     target/i386/cpu.c
>     target/i386/kvm/kvm.c
>     target/i386/tcg/seg_helper.c
>     target/i386/tcg/sysemu/svm_helper.c
>     target/i386/tcg/translate.c
>     target/m68k/translate.c

Likewise.

>     target/mips/tcg/msa_helper.c

Likewise.

>     target/mips/tcg/nanomips_translate.c.inc

Likewise.

>     target/mips/tcg/translate.c

Likewise.

>     target/ppc/int_helper.c
>     target/riscv/cpu.c
>     target/riscv/vector_helper.c
>     target/tricore/translate.c
>     tcg/tcg.c

Likewise.

>     tests/qtest/m48t59-test.c
>     tests/qtest/pflash-cfi02-test.c
>     tests/unit/test-throttle.c
>     util/vhost-user-server.c