[Qemu-devel] Headers without multiple inclusion guards

[Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

We have a bunch of headers without multiple inclusion guards.  Some are
clearly intentional, some look accidental.  Too many for me to find out
by examining each of them, so I'm asking their maintainers.

Why do I ask?  I'd like to mark the intentional ones and fix the
accidental ones, so they don't flunk "make check-headers" from "[RFC v4
0/7] Baby steps towards saner headers" just because they lack multiple
inclusion guards.

Just in case: what's a multiple inclusion guard?  It's

    #ifndef UNIQUE_GUARD_SYMBOL_H
    #define UNIQUE_GUARD_SYMBOL_H
    ...
    #endif

with nothing but comments outside the conditional, so that the header
can safely be included more than once.

I append the alphabetical list of headers without multiple inclusion
guards (as reported by scripts/clean-header-guards -nv), followed by the
same list sorted into maintainer buckets.  If you're cc'ed, please find
your bucket(s), and tell me which headers intentionally lack guards.


= Headers without inclusion guards =

accel/tcg/atomic_template.h
accel/tcg/tcg-runtime.h
audio/audio_template.h
audio/dsound_template.h
audio/mixeng_template.h
audio/rate_template.h
bsd-user/bsd-mman.h
bsd-user/errno_defs.h
bsd-user/freebsd/syscall_nr.h
bsd-user/netbsd/syscall_nr.h
bsd-user/openbsd/syscall_nr.h
bsd-user/syscall_defs.h
crypto/ivgen-essiv.h
crypto/ivgen-plain.h
crypto/ivgen-plain64.h
fpu/softfloat-specialize.h
hw/9pfs/xen-9pfs.h
hw/audio/hda-codec-common.h
hw/display/cirrus_vga_rop.h
hw/display/cirrus_vga_rop2.h
hw/display/milkymist-vgafb_template.h
hw/display/omap_lcd_template.h
hw/display/pl110_template.h
hw/display/pxa2xx_template.h
hw/display/sm501_template.h
hw/display/tc6393xb_template.h
hw/display/vga-helpers.h
hw/hppa/hppa_hardware.h
hw/input/adb-internal.h
hw/net/e1000e_core.h
hw/net/e1000x_common.h
hw/net/vmxnet3_defs.h
hw/usb/hcd-xhci.h
hw/usb/quirks-ftdi-ids.h
hw/usb/quirks-pl2303-ids.h
hw/usb/quirks.h
include/exec/cpu_ldst_template.h
include/exec/cpu_ldst_useronly_template.h
include/exec/memory_ldst.inc.h
include/exec/memory_ldst_cached.inc.h
include/exec/memory_ldst_phys.inc.h
include/fpu/softfloat-macros.h
include/hw/arm/allwinner-a10.h
include/hw/arm/omap.h
include/hw/arm/raspi_platform.h
include/hw/arm/xlnx-zynqmp.h
include/hw/char/cadence_uart.h
include/hw/display/xlnx_dp.h
include/hw/dma/xlnx-zynq-devcfg.h
include/hw/elf_ops.h
include/hw/kvm/clock.h
include/hw/m68k/mcf_fec.h
include/hw/mips/bios.h
include/hw/net/cadence_gem.h
include/hw/pci-bridge/simba.h
include/hw/pci/pci_regs.h
include/hw/qdev-dma.h
include/hw/sparc/sparc64.h
include/hw/timer/sun4v-rtc.h
include/hw/timer/xlnx-zynqmp-rtc.h
include/hw/xen/io/ring.h
include/libdecnumber/dconfig.h
include/libdecnumber/decDPD.h
include/libdecnumber/dpd/decimal128Local.h
include/ui/pixel_ops.h
include/ui/spice-display.h
linux-user/aarch64/sockbits.h
linux-user/aarch64/syscall_nr.h
linux-user/aarch64/termbits.h
linux-user/alpha/syscall_nr.h
linux-user/alpha/termbits.h
linux-user/arm/sockbits.h
linux-user/arm/syscall_nr.h
linux-user/arm/termbits.h
linux-user/cris/sockbits.h
linux-user/cris/syscall_nr.h
linux-user/cris/termbits.h
linux-user/errno_defs.h
linux-user/flat.h
linux-user/hppa/sockbits.h
linux-user/hppa/syscall_nr.h
linux-user/hppa/termbits.h
linux-user/i386/sockbits.h
linux-user/i386/syscall_nr.h
linux-user/i386/termbits.h
linux-user/ioctls.h
linux-user/m68k/sockbits.h
linux-user/m68k/syscall_nr.h
linux-user/m68k/termbits.h
linux-user/microblaze/sockbits.h
linux-user/microblaze/syscall_nr.h
linux-user/microblaze/termbits.h
linux-user/mips/syscall_nr.h
linux-user/mips/termbits.h
linux-user/mips64/sockbits.h
linux-user/mips64/syscall_nr.h
linux-user/mips64/target_cpu.h
linux-user/mips64/target_fcntl.h
linux-user/mips64/target_structs.h
linux-user/mips64/termbits.h
linux-user/nios2/sockbits.h
linux-user/nios2/syscall_nr.h
linux-user/nios2/termbits.h
linux-user/openrisc/sockbits.h
linux-user/openrisc/syscall_nr.h
linux-user/openrisc/termbits.h
linux-user/ppc/syscall_nr.h
linux-user/ppc/termbits.h
linux-user/riscv/sockbits.h
linux-user/riscv/syscall_nr.h
linux-user/riscv/target_syscall.h
linux-user/riscv/termbits.h
linux-user/s390x/sockbits.h
linux-user/s390x/syscall_nr.h
linux-user/s390x/termbits.h
linux-user/sh4/sockbits.h
linux-user/sh4/syscall_nr.h
linux-user/sh4/termbits.h
linux-user/socket.h
linux-user/sparc/syscall_nr.h
linux-user/sparc/termbits.h
linux-user/sparc64/sockbits.h
linux-user/sparc64/syscall_nr.h
linux-user/sparc64/target_cpu.h
linux-user/sparc64/target_fcntl.h
linux-user/sparc64/target_signal.h
linux-user/sparc64/termbits.h
linux-user/syscall_types.h
linux-user/target_flat.h
linux-user/tilegx/sockbits.h
linux-user/x86_64/sockbits.h
linux-user/x86_64/syscall_nr.h
linux-user/x86_64/target_cpu.h
linux-user/x86_64/termbits.h
linux-user/xtensa/sockbits.h
linux-user/xtensa/target_flat.h
qemu-options-wrapper.h
scripts/cocci-macro-file.h
target/alpha/helper.h
target/arm/helper-a64.h
target/arm/helper-sve.h
target/arm/helper.h
target/arm/op_addsub.h
target/cris/crisv10-decode.h
target/cris/helper.h
target/cris/mmu.h
target/cris/opcode-cris.h
target/hppa/helper.h
target/i386/cc_helper_template.h
target/i386/helper.h
target/i386/ops_sse.h
target/i386/ops_sse_header.h
target/i386/shift_helper_template.h
target/i386/whp-dispatch.h
target/lm32/helper.h
target/m68k/helper.h
target/microblaze/helper.h
target/microblaze/microblaze-decode.h
target/microblaze/mmu.h
target/mips/helper.h
target/moxie/helper.h
target/moxie/machine.h
target/moxie/mmu.h
target/nios2/helper.h
target/openrisc/helper.h
target/ppc/helper.h
target/riscv/cpu_bits.h
target/riscv/cpu_user.h
target/riscv/helper.h
target/riscv/instmap.h
target/s390x/helper.h
target/sh4/helper.h
target/sparc/helper.h
target/tilegx/helper.h
target/tilegx/spr_def_64.h
target/tricore/helper.h
target/tricore/tricore-opcodes.h
target/unicore32/helper.h
target/xtensa/helper.h
target/xtensa/overlay_tool.h
target/xtensa/xtensa-isa.h
tcg/aarch64/tcg-target.opc.h
tcg/i386/tcg-target.opc.h
tcg/tcg-gvec-desc.h
tcg/tcg-op-gvec.h
tcg/tcg-opc.h
tests/crypto-tls-psk-helpers.h
tests/crypto-tls-x509-helpers.h
tests/io-channel-helpers.h
tests/libqos/virtio-9p.h
tests/libqos/virtio-balloon.h
tests/libqos/virtio-blk.h
tests/libqos/virtio-net.h
tests/libqos/virtio-rng.h
tests/libqos/virtio-scsi.h
tests/libqos/virtio-serial.h
tests/migration/aarch64/a-b-kernel.h
tests/migration/i386/a-b-bootblock.h
tests/migration/s390x/a-b-bios.h
tests/socket-helpers.h
tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
ui/vgafont.h
ui/vnc-enc-hextile-template.h
ui/vnc_keysym.h


= Sorted into MAINTAINERS buckets =

Guest CPU cores (TCG):
----------------------
Overall
M: Richard Henderson <rth@twiddle.net>
R: Paolo Bonzini <pbonzini@redhat.com>
accel/tcg/atomic_template.h
accel/tcg/tcg-runtime.h
include/exec/cpu_ldst_template.h
include/exec/cpu_ldst_useronly_template.h

FPU emulation
M: Aurelien Jarno <aurelien@aurel32.net>
M: Peter Maydell <peter.maydell@linaro.org>
M: Alex Bennée <alex.bennee@linaro.org>
fpu/softfloat-specialize.h
include/fpu/softfloat-macros.h

Alpha
M: Richard Henderson <rth@twiddle.net>

ARM
M: Peter Maydell <peter.maydell@linaro.org>
target/arm/helper-a64.h
target/arm/helper-sve.h
target/arm/helper.h
target/arm/op_addsub.h

ARM SMMU
M: Eric Auger <eric.auger@redhat.com>

CRIS
M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
target/cris/crisv10-decode.h
target/cris/helper.h
target/cris/mmu.h
target/cris/opcode-cris.h

HPPA (PA-RISC)
M: Richard Henderson <rth@twiddle.net>
target/hppa/helper.h

LM32
M: Michael Walle <michael@walle.cc>
hw/display/milkymist-vgafb_template.h
target/lm32/helper.h

M68K
M: Laurent Vivier <laurent@vivier.eu>
target/m68k/helper.h

MicroBlaze
M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
target/microblaze/helper.h
target/microblaze/microblaze-decode.h
target/microblaze/mmu.h

MIPS
M: Aurelien Jarno <aurelien@aurel32.net>
M: Aleksandar Markovic <amarkovic@wavecomp.com>
R: Aleksandar Rikalo <arikalo@wavecomp.com>
include/hw/mips/bios.h
target/mips/helper.h

Moxie
M: Anthony Green <green@moxielogic.com>
target/moxie/helper.h
target/moxie/machine.h
target/moxie/mmu.h

NiosII
M: Chris Wulff <crwulff@gmail.com>
M: Marek Vasut <marex@denx.de>
target/nios2/helper.h

OpenRISC
M: Stafford Horne <shorne@gmail.com>
target/openrisc/helper.h

PowerPC
M: David Gibson <david@gibson.dropbear.id.au>
target/ppc/helper.h

RISC-V
M: Palmer Dabbelt <palmer@sifive.com>
M: Alistair Francis <Alistair.Francis@wdc.com>
M: Sagar Karandikar <sagark@eecs.berkeley.edu>
M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
target/riscv/cpu_bits.h
target/riscv/cpu_user.h
target/riscv/helper.h
target/riscv/instmap.h

S390
M: Richard Henderson <rth@twiddle.net>
M: David Hildenbrand <david@redhat.com>
target/s390x/helper.h

SH4
M: Aurelien Jarno <aurelien@aurel32.net>
target/sh4/helper.h

SPARC
M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
M: Artyom Tarasenko <atar4qemu@gmail.com>
include/hw/sparc/sparc64.h
target/sparc/helper.h

UniCore32
M: Guan Xuetao <gxt@mprc.pku.edu.cn>
target/unicore32/helper.h

X86
M: Paolo Bonzini <pbonzini@redhat.com>
M: Richard Henderson <rth@twiddle.net>
M: Eduardo Habkost <ehabkost@redhat.com>
target/i386/cc_helper_template.h
target/i386/helper.h
target/i386/ops_sse.h
target/i386/ops_sse_header.h
target/i386/shift_helper_template.h
target/i386/whp-dispatch.h

Xtensa
M: Max Filippov <jcmvbkbc@gmail.com>
target/xtensa/helper.h
target/xtensa/overlay_tool.h
target/xtensa/xtensa-isa.h

TriCore
M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
target/tricore/helper.h
target/tricore/tricore-opcodes.h

Multiarch Linux User Tests
M: Alex Bennée <alex.bennee@linaro.org>

Guest CPU Cores (KVM):
----------------------

Overall
M: Paolo Bonzini <pbonzini@redhat.com>
include/hw/kvm/clock.h

ARM
M: Peter Maydell <peter.maydell@linaro.org>

MIPS
M: James Hogan <jhogan@kernel.org>
R: Aleksandar Rikalo <arikalo@wavecomp.com>

PPC
M: David Gibson <david@gibson.dropbear.id.au>

S390
M: Halil Pasic <pasic@linux.ibm.com>
M: Cornelia Huck <cohuck@redhat.com>
M: Christian Borntraeger <borntraeger@de.ibm.com>

X86
M: Paolo Bonzini <pbonzini@redhat.com>
M: Marcelo Tosatti <mtosatti@redhat.com>

Guest CPU Cores (Xen):
----------------------

X86
M: Stefano Stabellini <sstabellini@kernel.org>
M: Anthony Perard <anthony.perard@citrix.com>
M: Paul Durrant <paul.durrant@citrix.com>
include/hw/xen/io/ring.h

ARM Machines
------------
Allwinner-a10
M: Beniamino Galvani <b.galvani@gmail.com>
M: Peter Maydell <peter.maydell@linaro.org>
include/hw/arm/allwinner-a10.h

ARM PrimeCell and CMSDK devices
M: Peter Maydell <peter.maydell@linaro.org>
hw/display/pl110_template.h

Raspberry Pi
M: Peter Maydell <peter.maydell@linaro.org>
R: Andrew Baumann <Andrew.Baumann@microsoft.com>
R: Philippe Mathieu-Daudé <f4bug@amsat.org>
include/hw/arm/raspi_platform.h

PXA2XX
M: Andrzej Zaborowski <balrogg@gmail.com>
M: Peter Maydell <peter.maydell@linaro.org>
hw/display/pxa2xx_template.h

Xilinx ZynqMP
M: Alistair Francis <alistair@alistair23.me>
M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
M: Peter Maydell <peter.maydell@linaro.org>
include/hw/arm/xlnx-zynqmp.h
include/hw/display/xlnx_dp.h
include/hw/dma/xlnx-zynq-devcfg.h
include/hw/timer/xlnx-zynqmp-rtc.h

HP-PARISC Machines
------------------
Dino
M: Richard Henderson <rth@twiddle.net>
R: Helge Deller <deller@gmx.de>
hw/hppa/hppa_hardware.h

PowerPC Machines
----------------
New World (mac99)
M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
R: David Gibson <david@gibson.dropbear.id.au>
hw/input/adb-internal.h

sam460ex
M: BALATON Zoltan <balaton@eik.bme.hu>
hw/display/sm501_template.h

SPARC Machines
--------------
Sun4u
M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
include/hw/pci-bridge/simba.h

Sun4v
M: Artyom Tarasenko <atar4qemu@gmail.com>
include/hw/timer/sun4v-rtc.h

Devices
-------

OMAP
M: Peter Maydell <peter.maydell@linaro.org>
hw/display/omap_lcd_template.h
include/hw/arm/omap.h

PCI
M: Michael S. Tsirkin <mst@redhat.com>
M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
include/hw/pci/pci_regs.h

Network devices
M: Jason Wang <jasowang@redhat.com>
hw/net/e1000e_core.h
hw/net/e1000x_common.h
hw/net/vmxnet3_defs.h
include/hw/net/cadence_gem.h

USB
M: Gerd Hoffmann <kraxel@redhat.com>
hw/usb/hcd-xhci.h
hw/usb/quirks-ftdi-ids.h
hw/usb/quirks-pl2303-ids.h
hw/usb/quirks.h

virtio-9p
M: Greg Kurz <groug@kaod.org>
hw/9pfs/xen-9pfs.h

Vmware
M: Dmitry Fleytman <dmitry.fleytman@gmail.com>
hw/net/vmxnet3_defs.h

e1000x
M: Dmitry Fleytman <dmitry.fleytman@gmail.com>
hw/net/e1000x_common.h

e1000e
M: Dmitry Fleytman <dmitry.fleytman@gmail.com>
hw/net/e1000e_core.h

Cirrus VGA
M: Gerd Hoffmann <kraxel@redhat.com>
hw/display/cirrus_vga_rop.h
hw/display/cirrus_vga_rop2.h

Subsystems
----------
Audio
M: Gerd Hoffmann <kraxel@redhat.com>
audio/audio_template.h
audio/dsound_template.h
audio/mixeng_template.h
audio/rate_template.h
hw/audio/hda-codec-common.h

SPICE
M: Gerd Hoffmann <kraxel@redhat.com>
include/ui/spice-display.h

Graphics
M: Gerd Hoffmann <kraxel@redhat.com>
include/ui/pixel_ops.h
ui/vgafont.h
ui/vnc-enc-hextile-template.h
ui/vnc_keysym.h

qtest
M: Thomas Huth <thuth@redhat.com>
M: Laurent Vivier <lvivier@redhat.com>
R: Paolo Bonzini <pbonzini@redhat.com>
tests/libqos/virtio-9p.h
tests/libqos/virtio-balloon.h
tests/libqos/virtio-blk.h
tests/libqos/virtio-net.h
tests/libqos/virtio-rng.h
tests/libqos/virtio-scsi.h
tests/libqos/virtio-serial.h

Cryptography
M: Daniel P. Berrange <berrange@redhat.com>
crypto/ivgen-essiv.h
crypto/ivgen-plain.h
crypto/ivgen-plain64.h
tests/crypto-tls-psk-helpers.h
tests/crypto-tls-x509-helpers.h

EDK2 Firmware
M: Laszlo Ersek <lersek@redhat.com>
M: Philippe Mathieu-Daudé <philmd@redhat.com>
tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h

Usermode Emulation
------------------
BSD user
S: Orphan
bsd-user/bsd-mman.h
bsd-user/errno_defs.h
bsd-user/freebsd/syscall_nr.h
bsd-user/netbsd/syscall_nr.h
bsd-user/openbsd/syscall_nr.h
bsd-user/syscall_defs.h

Linux user
M: Riku Voipio <riku.voipio@iki.fi>
R: Laurent Vivier <laurent@vivier.eu>
linux-user/aarch64/sockbits.h
linux-user/aarch64/syscall_nr.h
linux-user/aarch64/termbits.h
linux-user/alpha/syscall_nr.h
linux-user/alpha/termbits.h
linux-user/arm/sockbits.h
linux-user/arm/syscall_nr.h
linux-user/arm/termbits.h
linux-user/cris/sockbits.h
linux-user/cris/syscall_nr.h
linux-user/cris/termbits.h
linux-user/errno_defs.h
linux-user/flat.h
linux-user/hppa/sockbits.h
linux-user/hppa/syscall_nr.h
linux-user/hppa/termbits.h
linux-user/i386/sockbits.h
linux-user/i386/syscall_nr.h
linux-user/i386/termbits.h
linux-user/ioctls.h
linux-user/m68k/sockbits.h
linux-user/m68k/syscall_nr.h
linux-user/m68k/termbits.h
linux-user/microblaze/sockbits.h
linux-user/microblaze/syscall_nr.h
linux-user/microblaze/termbits.h
linux-user/mips/syscall_nr.h
linux-user/mips/termbits.h
linux-user/mips64/sockbits.h
linux-user/mips64/syscall_nr.h
linux-user/mips64/target_cpu.h
linux-user/mips64/target_fcntl.h
linux-user/mips64/target_structs.h
linux-user/mips64/termbits.h
linux-user/nios2/sockbits.h
linux-user/nios2/syscall_nr.h
linux-user/nios2/termbits.h
linux-user/openrisc/sockbits.h
linux-user/openrisc/syscall_nr.h
linux-user/openrisc/termbits.h
linux-user/ppc/syscall_nr.h
linux-user/ppc/termbits.h
linux-user/riscv/sockbits.h
linux-user/riscv/syscall_nr.h
linux-user/riscv/target_syscall.h
linux-user/riscv/termbits.h
linux-user/s390x/sockbits.h
linux-user/s390x/syscall_nr.h
linux-user/s390x/termbits.h
linux-user/sh4/sockbits.h
linux-user/sh4/syscall_nr.h
linux-user/sh4/termbits.h
linux-user/socket.h
linux-user/sparc/syscall_nr.h
linux-user/sparc/termbits.h
linux-user/sparc64/sockbits.h
linux-user/sparc64/syscall_nr.h
linux-user/sparc64/target_cpu.h
linux-user/sparc64/target_fcntl.h
linux-user/sparc64/target_signal.h
linux-user/sparc64/termbits.h
linux-user/syscall_types.h
linux-user/target_flat.h
linux-user/tilegx/sockbits.h
linux-user/x86_64/sockbits.h
linux-user/x86_64/syscall_nr.h
linux-user/x86_64/target_cpu.h
linux-user/x86_64/termbits.h
linux-user/xtensa/sockbits.h
linux-user/xtensa/target_flat.h

Tiny Code Generator (TCG)
-------------------------
Common code
M: Richard Henderson <rth@twiddle.net>
tcg/aarch64/tcg-target.opc.h
tcg/tcg-gvec-desc.h
tcg/tcg-op-gvec.h
tcg/tcg-opc.h

AArch64 target
M: Claudio Fontana <claudio.fontana@huawei.com>
M: Claudio Fontana <claudio.fontana@gmail.com>
tcg/aarch64/tcg-target.opc.h

i386 target
M: Richard Henderson <rth@twiddle.net>
tcg/i386/tcg-target.opc.h

UNMAINTAINED
------------
hw/display/tc6393xb_template.h
include/exec/memory_ldst.inc.h
include/exec/memory_ldst_cached.inc.h
include/exec/memory_ldst_phys.inc.h
include/hw/char/cadence_uart.h
include/hw/elf_ops.h
include/hw/qdev-dma.h
include/libdecnumber/dconfig.h
include/libdecnumber/decDPD.h
include/libdecnumber/dpd/decimal128Local.h
qemu-options-wrapper.h
scripts/cocci-macro-file.h
target/tilegx/helper.h
target/tilegx/spr_def_64.h
tests/io-channel-helpers.h
tests/migration/aarch64/a-b-kernel.h
tests/migration/i386/a-b-bootblock.h
tests/migration/s390x/a-b-bios.h
tests/socket-helpers.h

Re: [Qemu-devel] Headers without multiple inclusion guards

[Peter Maydell]

On Tue, 28 May 2019 at 19:12, Markus Armbruster <armbru@redhat.com> wrote:
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.

> FPU emulation
> M: Aurelien Jarno <aurelien@aurel32.net>
> M: Peter Maydell <peter.maydell@linaro.org>
> M: Alex Bennée <alex.bennee@linaro.org>
> fpu/softfloat-specialize.h
> include/fpu/softfloat-macros.h

I think that include/fpu/softfloat-macros.h should ideally
have header guards.

fpu/softfloat-specialize.h is a bit odd as it's intended to be included
exactly once, by fpu/softfloat.c. Guards wouldn't hurt, I think, but it
might be cleanest to rename it to fpu/softfloat-specialize.inc.c
to match our convention for "lump of C which is textually
included but isn't a header".

> ARM
> M: Peter Maydell <peter.maydell@linaro.org>
> target/arm/helper-a64.h
> target/arm/helper-sve.h
> target/arm/helper.h
> target/arm/op_addsub.h

All intentional -- target/*/helper.h and anything it includes
is intended to be included multiple times for different
purposes (the includer #defines various macros appropriately
to produce different effects).

> Allwinner-a10
> M: Beniamino Galvani <b.galvani@gmail.com>
> M: Peter Maydell <peter.maydell@linaro.org>
> include/hw/arm/allwinner-a10.h

This appears to me to have an include guard:
perhaps your check doesn't recognize the variant
where the #define appears just before the #endif rather
than just after the #ifndef ?

> ARM PrimeCell and CMSDK devices
> M: Peter Maydell <peter.maydell@linaro.org>
> hw/display/pl110_template.h

Intentional, like all the hw/display/*_template.h headers:
they're included multiple times with various definitions
of BITS to generate different functions. (Note that there's
an item on the BiteSizedTasks page which suggests that
we could clean up all of these because only the 32-bit
case is used now that our display surfaces are always
32-bits-per-pixel.)

> Raspberry Pi
> M: Peter Maydell <peter.maydell@linaro.org>
> R: Andrew Baumann <Andrew.Baumann@microsoft.com>
> R: Philippe Mathieu-Daudé <f4bug@amsat.org>
> include/hw/arm/raspi_platform.h

Mistake, should have a guard.

> PXA2XX
> M: Andrzej Zaborowski <balrogg@gmail.com>
> M: Peter Maydell <peter.maydell@linaro.org>
> hw/display/pxa2xx_template.h

Intentional, see above.

> Xilinx ZynqMP
> M: Alistair Francis <alistair@alistair23.me>
> M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> M: Peter Maydell <peter.maydell@linaro.org>
> include/hw/arm/xlnx-zynqmp.h
> include/hw/display/xlnx_dp.h
> include/hw/dma/xlnx-zynq-devcfg.h

These all have guards in non-standard formats.

> include/hw/timer/xlnx-zynqmp-rtc.h

Mistake, should have a guard.

> OMAP
> M: Peter Maydell <peter.maydell@linaro.org>
> hw/display/omap_lcd_template.h

Intentional, see above.

> include/hw/arm/omap.h

Has a guard in a non-standard form.

> UNMAINTAINED
> ------------
> hw/display/tc6393xb_template.h

Intentional (see above)

> include/hw/char/cadence_uart.h

Another with a guard which isn't of a style your check
detects.

thanks
-- PMM

Re: [Qemu-devel] Headers without multiple inclusion guards

[Eduardo Habkost]

On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
> 
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.
> 
[...]
> X86
> M: Paolo Bonzini <pbonzini@redhat.com>
> M: Richard Henderson <rth@twiddle.net>
> M: Eduardo Habkost <ehabkost@redhat.com>
> target/i386/cc_helper_template.h

Intentional.  See usage at target/i386/cc_helper.c.

> target/i386/helper.h

I believe helper.h intentionally lack guards on all
architectures.  See helper-proto.h, helper-tcg.h,
helper-gen.h.

> target/i386/ops_sse.h

Intentional, see usage at target/i386/fpu_helper.c.

> target/i386/ops_sse_header.h

Intentional, see usage at target/i386/helper.h.

> target/i386/shift_helper_template.h

Intentional, see usage at target/i386/int_helper.c.

> target/i386/whp-dispatch.h

Seems unintentional.

[...]
> Guest CPU Cores (KVM):
> ----------------------
> 
> Overall
> M: Paolo Bonzini <pbonzini@redhat.com>
> include/hw/kvm/clock.h

Seems unintentional.

[...]
> Guest CPU Cores (Xen):
> ----------------------
> 
> X86
> M: Stefano Stabellini <sstabellini@kernel.org>
> M: Anthony Perard <anthony.perard@citrix.com>
> M: Paul Durrant <paul.durrant@citrix.com>
> include/hw/xen/io/ring.h

I see a __XEN_PUBLIC_IO_RING_H__ guard there.  Probably
clean-header-guards.pl is confused by the comments at the end of
the file?

> [...]

-- 
Eduardo

Re: [Qemu-devel] Headers without multiple inclusion guards

[Max Filippov]

On Tue, May 28, 2019 at 11:12 AM Markus Armbruster <armbru@redhat.com> wrote:
> target/xtensa/helper.h
Intentional.

> target/xtensa/overlay_tool.h
Unintentional.

> target/xtensa/xtensa-isa.h
It's a one-liner that includes another header.

-- 
Thanks.
-- Max

Re: [Qemu-devel] Headers without multiple inclusion guards

[Richard Henderson]

On 5/28/19 1:12 PM, Markus Armbruster wrote:
> accel/tcg/atomic_template.h

Intentional; could be renamed atomic_template.inc.c.

> accel/tcg/tcg-runtime.h

Intentional.

> tcg/aarch64/tcg-target.opc.h
> tcg/i386/tcg-target.opc.h

Intentional.

> tcg/tcg-gvec-desc.h
> tcg/tcg-op-gvec.h

Not intentional.

> tcg/tcg-opc.h

Intentional.


r~

Re: [Qemu-devel] Headers without multiple inclusion guards

[BALATON Zoltan]

On Tue, 28 May 2019, Markus Armbruster wrote:
> sam460ex
> M: BALATON Zoltan <balaton@eik.bme.hu>
> hw/display/sm501_template.h

This is like other *_template.h files mentioned by Peter in his reply and 
is intended to be included multiple times.

Regards,
BALATON Zoltan

Re: [Qemu-devel] Headers without multiple inclusion guards

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
> 
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.

[snip]
> PowerPC
> M: David Gibson <david@gibson.dropbear.id.au>
> target/ppc/helper.h

Like the equivalent ARM variants, intentional.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laurent Vivier]

On 28/05/2019 20:12, Markus Armbruster wrote:
...
> Linux user
> M: Riku Voipio <riku.voipio@iki.fi>
> R: Laurent Vivier <laurent@vivier.eu>

Unintentionnal:

linux-user/errno_defs.h
linux-user/flat.h
linux-user/hppa/sockbits.h
linux-user/socket.h
linux-user/aarch64/syscall_nr.h
linux-user/alpha/syscall_nr.h
linux-user/arm/syscall_nr.h
linux-user/cris/syscall_nr.h
linux-user/hppa/syscall_nr.h
linux-user/i386/syscall_nr.h
linux-user/m68k/syscall_nr.h
linux-user/microblaze/syscall_nr.h
linux-user/mips/syscall_nr.h
linux-user/mips64/syscall_nr.h
linux-user/nios2/syscall_nr.h
linux-user/openrisc/syscall_nr.h
linux-user/ppc/syscall_nr.h
linux-user/riscv/syscall_nr.h
linux-user/s390x/syscall_nr.h
linux-user/sh4/syscall_nr.h
linux-user/sparc/syscall_nr.h
linux-user/sparc64/syscall_nr.h
linux-user/x86_64/syscall_nr.h
linux-user/target_flat.h
linux-user/xtensa/target_flat.h
linux-user/riscv/target_syscall.h
linux-user/aarch64/termbits.h
linux-user/alpha/termbits.h
linux-user/arm/termbits.h
linux-user/cris/termbits.h
linux-user/hppa/termbits.h
linux-user/i386/termbits.h
linux-user/m68k/termbits.h
linux-user/microblaze/termbits.h
linux-user/mips/termbits.h
linux-user/nios2/termbits.h
linux-user/openrisc/termbits.h
linux-user/ppc/termbits.h
linux-user/riscv/termbits.h
linux-user/s390x/termbits.h
linux-user/sh4/termbits.h
linux-user/sparc/termbits.h
linux-user/sparc64/termbits.h
linux-user/x86_64/termbits.h

Intentionnal:

linux-user/ioctls.h
linux-user/aarch64/sockbits.h
linux-user/arm/sockbits.h
linux-user/cris/sockbits.h
linux-user/i386/sockbits.h
linux-user/m68k/sockbits.h
linux-user/microblaze/sockbits.h
linux-user/mips64/sockbits.h
linux-user/nios2/sockbits.h
linux-user/openrisc/sockbits.h
linux-user/riscv/sockbits.h
linux-user/s390x/sockbits.h
linux-user/sh4/sockbits.h
linux-user/sparc64/sockbits.h
linux-user/tilegx/sockbits.h
linux-user/x86_64/sockbits.h
linux-user/xtensa/sockbits.h
linux-user/syscall_types.h
linux-user/mips64/target_cpu.h
linux-user/sparc64/target_cpu.h
linux-user/x86_64/target_cpu.h
linux-user/mips64/target_fcntl.h
linux-user/sparc64/target_fcntl.h
linux-user/sparc64/target_signal.h
linux-user/mips64/target_structs.h
linux-user/mips64/termbits.h

Thanks,
Laurent

Re: [Qemu-devel] Headers without multiple inclusion guards

[Paul Durrant]

 
> [...]
> > Guest CPU Cores (Xen):
> > ----------------------
> >
> > X86
> > M: Stefano Stabellini <sstabellini@kernel.org>
> > M: Anthony Perard <anthony.perard@citrix.com>
> > M: Paul Durrant <paul.durrant@citrix.com>
> > include/hw/xen/io/ring.h
> 
> I see a __XEN_PUBLIC_IO_RING_H__ guard there.  Probably
> clean-header-guards.pl is confused by the comments at the end of
> the file?
> 

Yes, I see no problem there. The emacs boilerplate is common in Xen headers and persisted when this was imported.

  Paul

> > [...]
> 
> --
> Eduardo

Re: [Qemu-devel] Headers without multiple inclusion guards

[Bastian Koppelmann]

On 5/28/19 8:12 PM, Markus Armbruster wrote:
> target/tricore/tricore-opcodes.h
unintentional.


Cheers,

Bastian

Re: [Qemu-devel] Headers without multiple inclusion guards

[Cornelia Huck]

On Tue, 28 May 2019 20:12:24 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
> 
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.

(...)

> S390
> M: Richard Henderson <rth@twiddle.net>
> M: David Hildenbrand <david@redhat.com>
> target/s390x/helper.h

Intentional, as all target/*/helper.h.

(...)

> UNMAINTAINED
> ------------

(...)

> tests/migration/s390x/a-b-bios.h

Autogenerated from a-b-bios.c, not intended to be included except in
the migration test.

[Side note: I probably should include tests/migration/s390x/ to the
s390 section in MAINTAINERS so it is no longer 'unmaintained'...]

Re: [Qemu-devel] Headers without multiple inclusion guards

[Greg Kurz]

On Tue, 28 May 2019 20:12:24 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
> 
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.
> 
[...]
> virtio-9p
> M: Greg Kurz <groug@kaod.org>
> hw/9pfs/xen-9pfs.h
> 

It doesn't seem to be intentional, but probably better to
ask Stefano Stabellini, Paul Durrant or Anthony Perard
since this is a xen-9p header, not virtio-9p.

In the meantime, I'll send a fix for MAINTAINERS.

Cheers,

--
Greg

Re: [Qemu-devel] Headers without multiple inclusion guards

[Alex Bennée]

Markus Armbruster <armbru@redhat.com> writes:

> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
>
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
>
> Just in case: what's a multiple inclusion guard?  It's
>
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
>
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
>
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.
>
>
> = Headers without inclusion guards =
<snip>
> include/exec/cpu_ldst_template.h
> include/exec/cpu_ldst_useronly_template.h
> include/exec/memory_ldst.inc.h
> include/exec/memory_ldst_cached.inc.h
> include/exec/memory_ldst_phys.inc.h

These are all included multiple times with different #define's to expand
out our various helpers. Eventually I hope to make them go away but for
now they are fine.

--
Alex Bennée

Re: [Qemu-devel] Headers without multiple inclusion guards

[Anthony PERARD]

On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.


> Guest CPU Cores (Xen):
> ----------------------
> 
> X86
> M: Stefano Stabellini <sstabellini@kernel.org>
> M: Anthony Perard <anthony.perard@citrix.com>
> M: Paul Durrant <paul.durrant@citrix.com>
> include/hw/xen/io/ring.h

That file does have a guard, it's __XEN_PUBLIC_IO_RING_H__

Do you (or the checker) want it to be changed to XEN_PUBLIC_IO_RING_H ?

> virtio-9p
> M: Greg Kurz <groug@kaod.org>
> hw/9pfs/xen-9pfs.h

Stefano, is the missing inclusion guard was intentional? Otherwise, I
can send a patch for it.

Cheers,

-- 
Anthony PERARD

Re: [Qemu-devel] Headers without multiple inclusion guards

[David Hildenbrand]

> 
> S390
> M: Richard Henderson <rth@twiddle.net>
> M: David Hildenbrand <david@redhat.com>
> target/s390x/helper.h

Intentional.

Cheers!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 28 May 2019 at 19:12, Markus Armbruster <armbru@redhat.com> wrote:
>> I append the alphabetical list of headers without multiple inclusion
>> guards (as reported by scripts/clean-header-guards -nv), followed by the
>> same list sorted into maintainer buckets.  If you're cc'ed, please find
>> your bucket(s), and tell me which headers intentionally lack guards.
>
>> FPU emulation
>> M: Aurelien Jarno <aurelien@aurel32.net>
>> M: Peter Maydell <peter.maydell@linaro.org>
>> M: Alex Bennée <alex.bennee@linaro.org>
>> fpu/softfloat-specialize.h
>> include/fpu/softfloat-macros.h
>
> I think that include/fpu/softfloat-macros.h should ideally
> have header guards.

I'll add one.

> fpu/softfloat-specialize.h is a bit odd as it's intended to be included
> exactly once, by fpu/softfloat.c. Guards wouldn't hurt, I think, but it
> might be cleanest to rename it to fpu/softfloat-specialize.inc.c
> to match our convention for "lump of C which is textually
> included but isn't a header".

Makes me wonder why we bother with a separate file.

Renaming it to .inc.c gets it out of check-headers' way.

>> ARM
>> M: Peter Maydell <peter.maydell@linaro.org>
>> target/arm/helper-a64.h
>> target/arm/helper-sve.h
>> target/arm/helper.h
>> target/arm/op_addsub.h
>
> All intentional -- target/*/helper.h and anything it includes
> is intended to be included multiple times for different
> purposes (the includer #defines various macros appropriately
> to produce different effects).

We have quite a few special headers of this kind.  The ones that require
macros to be defined should be excluded from check-headers.  The ones
that don't could instead skip the "#include a second time works" part of
the test.  We'll see.

>> Allwinner-a10
>> M: Beniamino Galvani <b.galvani@gmail.com>
>> M: Peter Maydell <peter.maydell@linaro.org>
>> include/hw/arm/allwinner-a10.h
>
> This appears to me to have an include guard:
> perhaps your check doesn't recognize the variant
> where the #define appears just before the #endif rather
> than just after the #ifndef ?

You're right, it doesn't.

We got a dozen headers with the #define in unusual places.  I'll
normalize them.

>> ARM PrimeCell and CMSDK devices
>> M: Peter Maydell <peter.maydell@linaro.org>
>> hw/display/pl110_template.h
>
> Intentional, like all the hw/display/*_template.h headers:
> they're included multiple times with various definitions
> of BITS to generate different functions. (Note that there's
> an item on the BiteSizedTasks page which suggests that
> we could clean up all of these because only the 32-bit
> case is used now that our display surfaces are always
> 32-bits-per-pixel.)

Another one for the "special headers" bucket.

>> Raspberry Pi
>> M: Peter Maydell <peter.maydell@linaro.org>
>> R: Andrew Baumann <Andrew.Baumann@microsoft.com>
>> R: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> include/hw/arm/raspi_platform.h
>
> Mistake, should have a guard.

I'll add one.

>> PXA2XX
>> M: Andrzej Zaborowski <balrogg@gmail.com>
>> M: Peter Maydell <peter.maydell@linaro.org>
>> hw/display/pxa2xx_template.h
>
> Intentional, see above.

Another one for the "special headers" bucket.

>> Xilinx ZynqMP
>> M: Alistair Francis <alistair@alistair23.me>
>> M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>> M: Peter Maydell <peter.maydell@linaro.org>
>> include/hw/arm/xlnx-zynqmp.h
>> include/hw/display/xlnx_dp.h
>> include/hw/dma/xlnx-zynq-devcfg.h
>
> These all have guards in non-standard formats.

I'll normalize.

>> include/hw/timer/xlnx-zynqmp-rtc.h
>
> Mistake, should have a guard.

I'll add one.

>> OMAP
>> M: Peter Maydell <peter.maydell@linaro.org>
>> hw/display/omap_lcd_template.h
>
> Intentional, see above.

Another one for the "special headers" bucket.

>> include/hw/arm/omap.h
>
> Has a guard in a non-standard form.

I'll normalize.

>> UNMAINTAINED
>> ------------
>> hw/display/tc6393xb_template.h
>
> Intentional (see above)

Another one for the "special headers" bucket.

>> include/hw/char/cadence_uart.h
>
> Another with a guard which isn't of a style your check
> detects.

I'll normalize.

Thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Paul Durrant <Paul.Durrant@citrix.com> writes:

>  
>> [...]
>> > Guest CPU Cores (Xen):
>> > ----------------------
>> >
>> > X86
>> > M: Stefano Stabellini <sstabellini@kernel.org>
>> > M: Anthony Perard <anthony.perard@citrix.com>
>> > M: Paul Durrant <paul.durrant@citrix.com>
>> > include/hw/xen/io/ring.h
>> 
>> I see a __XEN_PUBLIC_IO_RING_H__ guard there.  Probably
>> clean-header-guards.pl is confused by the comments at the end of
>> the file?
>> 
>
> Yes, I see no problem there. The emacs boilerplate is common in Xen headers and persisted when this was imported.

I fixed scripts/clean-header-guards.pl in my tree to cope with this
pattern.

Thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Max Filippov <jcmvbkbc@gmail.com> writes:

> On Tue, May 28, 2019 at 11:12 AM Markus Armbruster <armbru@redhat.com> wrote:
>> target/xtensa/helper.h
> Intentional.
>
>> target/xtensa/overlay_tool.h
> Unintentional.
>
>> target/xtensa/xtensa-isa.h
> It's a one-liner that includes another header.

Feels like a bad idea, but it doesn't bother me right now.

Thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Anthony PERARD <anthony.perard@citrix.com> writes:

> On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
>> Just in case: what's a multiple inclusion guard?  It's
>> 
>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>     #define UNIQUE_GUARD_SYMBOL_H
>>     ...
>>     #endif
>> 
>> with nothing but comments outside the conditional, so that the header
>> can safely be included more than once.
>
>
>> Guest CPU Cores (Xen):
>> ----------------------
>> 
>> X86
>> M: Stefano Stabellini <sstabellini@kernel.org>
>> M: Anthony Perard <anthony.perard@citrix.com>
>> M: Paul Durrant <paul.durrant@citrix.com>
>> include/hw/xen/io/ring.h
>
> That file does have a guard, it's __XEN_PUBLIC_IO_RING_H__
>
> Do you (or the checker) want it to be changed to XEN_PUBLIC_IO_RING_H ?

Yes.  I'll post a patch.

>> virtio-9p
>> M: Greg Kurz <groug@kaod.org>
>> hw/9pfs/xen-9pfs.h
>
> Stefano, is the missing inclusion guard was intentional? Otherwise, I
> can send a patch for it.

Sending a patch is okay, but telling me "unintential, please fix it
along with the other ones" is also okay :)

Thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Philippe Mathieu-Daudé]

On 5/29/19 2:51 PM, Markus Armbruster wrote:
> Max Filippov <jcmvbkbc@gmail.com> writes:
> 
>> On Tue, May 28, 2019 at 11:12 AM Markus Armbruster <armbru@redhat.com> wrote:
>>> target/xtensa/helper.h
>> Intentional.
>>
>>> target/xtensa/overlay_tool.h
>> Unintentional.
>>
>>> target/xtensa/xtensa-isa.h
>> It's a one-liner that includes another header.
> 
> Feels like a bad idea, but it doesn't bother me right now.

I think include/hw/xtensa/xtensa-isa.h is misplaced, it should be in
target/xtensa/xtensa-isa.h, but since disas/xtensa.c includes it, it
ended in include/.

Re: [Qemu-devel] Headers without multiple inclusion guards

[Philippe Mathieu-Daudé]

On 5/28/19 8:12 PM, Markus Armbruster wrote:
> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.
> 
> I append the alphabetical list of headers without multiple inclusion
> guards (as reported by scripts/clean-header-guards -nv), followed by the
> same list sorted into maintainer buckets.  If you're cc'ed, please find
> your bucket(s), and tell me which headers intentionally lack guards.
> 
[...]
> 
> EDK2 Firmware
> M: Laszlo Ersek <lersek@redhat.com>
> M: Philippe Mathieu-Daudé <philmd@redhat.com>
> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h

This file has a guard in non-standard formats:

#ifndef __BIOS_TABLES_TEST_H__
#define __BIOS_TABLES_TEST_H__
...

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/28/19 8:12 PM, Markus Armbruster wrote:
>> We have a bunch of headers without multiple inclusion guards.  Some are
>> clearly intentional, some look accidental.  Too many for me to find out
>> by examining each of them, so I'm asking their maintainers.
>> 
>> Why do I ask?  I'd like to mark the intentional ones and fix the
>> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
>> 0/7] Baby steps towards saner headers" just because they lack multiple
>> inclusion guards.
>> 
>> Just in case: what's a multiple inclusion guard?  It's
>> 
>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>     #define UNIQUE_GUARD_SYMBOL_H
>>     ...
>>     #endif
>> 
>> with nothing but comments outside the conditional, so that the header
>> can safely be included more than once.
>> 
>> I append the alphabetical list of headers without multiple inclusion
>> guards (as reported by scripts/clean-header-guards -nv), followed by the
>> same list sorted into maintainer buckets.  If you're cc'ed, please find
>> your bucket(s), and tell me which headers intentionally lack guards.
>> 
> [...]
>> 
>> EDK2 Firmware
>> M: Laszlo Ersek <lersek@redhat.com>
>> M: Philippe Mathieu-Daudé <philmd@redhat.com>
>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
>
> This file has a guard in non-standard formats:
>
> #ifndef __BIOS_TABLES_TEST_H__
> #define __BIOS_TABLES_TEST_H__
> ...

scripts/clean-header-guards.pl didn't recognize the guard due to the //
comment after the #endif.  I fixed the script, then used it to clean up
this header.

Thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Peter Maydell]

On Wed, 29 May 2019 at 13:48, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > fpu/softfloat-specialize.h is a bit odd as it's intended to be included
> > exactly once, by fpu/softfloat.c. Guards wouldn't hurt, I think, but it
> > might be cleanest to rename it to fpu/softfloat-specialize.inc.c
> > to match our convention for "lump of C which is textually
> > included but isn't a header".
>
> Makes me wonder why we bother with a separate file.

Partly historical, because all the fpu/ code is originally
third-party from the softfloat project. Partly it separates
out "code which may need to be adjusted with ifdefs to
implement target-specific behaviour" from "code that should
be target-agnostic".

thanks
-- PMM

Re: [Qemu-devel] Headers without multiple inclusion guards

[Dmitry Fleytman]

> 
> hw/net/e1000e_core.h
> hw/net/e1000x_common.h
> hw/net/vmxnet3_defs.h


Unintentional.

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laszlo Ersek]

Hi Markus,

(sorry about the late reply, I've been away.)

On 05/28/19 20:12, Markus Armbruster wrote:

> EDK2 Firmware
> M: Laszlo Ersek <lersek@redhat.com>
> M: Philippe Mathieu-Daudé <philmd@redhat.com>
> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h

This header file does have a multiple inclusion guard:

> /** @file
>   Expose the address(es) of the ACPI RSD PTR table(s) and the SMBIOS entry
>   point(s) in a MB-aligned structure to the hypervisor.
>
>   [...]
> **/
>
> #ifndef __BIOS_TABLES_TEST_H__
> #define __BIOS_TABLES_TEST_H__
>
> [...]
>
> #endif // __BIOS_TABLES_TEST_H__

It's possible that "scripts/clean-header-guards.pl" does not recognize
the guard.

According to the ISO C standard, "All identifiers that begin with an
underscore and either an uppercase letter or another underscore are
always reserved for any use". Therefore, technically speaking, the above
inclusion guard implies undefined behavior. In practice, this particular
style for header guards is extremely common in the edk2 codebase:

$ git grep '^#ifndef __' -- '*.h'  | wc -l
1012

And, "tests/uefi-test-tools/UefiTestToolsPkg" follows the edk2 coding
style.

That said, if you'd like to remove the leading "__" from the macro name,
I'd be fully OK with that.

Thank you!
Laszlo

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laszlo Ersek]

On 05/29/19 16:10, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 5/28/19 8:12 PM, Markus Armbruster wrote:
>>> We have a bunch of headers without multiple inclusion guards.  Some are
>>> clearly intentional, some look accidental.  Too many for me to find out
>>> by examining each of them, so I'm asking their maintainers.
>>>
>>> Why do I ask?  I'd like to mark the intentional ones and fix the
>>> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
>>> 0/7] Baby steps towards saner headers" just because they lack multiple
>>> inclusion guards.
>>>
>>> Just in case: what's a multiple inclusion guard?  It's
>>>
>>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>>     #define UNIQUE_GUARD_SYMBOL_H
>>>     ...
>>>     #endif
>>>
>>> with nothing but comments outside the conditional, so that the header
>>> can safely be included more than once.
>>>
>>> I append the alphabetical list of headers without multiple inclusion
>>> guards (as reported by scripts/clean-header-guards -nv), followed by the
>>> same list sorted into maintainer buckets.  If you're cc'ed, please find
>>> your bucket(s), and tell me which headers intentionally lack guards.
>>>
>> [...]
>>>
>>> EDK2 Firmware
>>> M: Laszlo Ersek <lersek@redhat.com>
>>> M: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
>>
>> This file has a guard in non-standard formats:
>>
>> #ifndef __BIOS_TABLES_TEST_H__
>> #define __BIOS_TABLES_TEST_H__
>> ...
> 
> scripts/clean-header-guards.pl didn't recognize the guard due to the //
> comment after the #endif.  I fixed the script, then used it to clean up
> this header.
> 
> Thanks!
> 

(Sorry, I've just replied to the thread starter, before seeing this
subthread. I'm OK if the header file is cleaned up.)

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Laszlo Ersek <lersek@redhat.com> writes:

> Hi Markus,
>
> (sorry about the late reply, I've been away.)
>
> On 05/28/19 20:12, Markus Armbruster wrote:
>
>> EDK2 Firmware
>> M: Laszlo Ersek <lersek@redhat.com>
>> M: Philippe Mathieu-Daudé <philmd@redhat.com>
>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
>
> This header file does have a multiple inclusion guard:
>
>> /** @file
>>   Expose the address(es) of the ACPI RSD PTR table(s) and the SMBIOS entry
>>   point(s) in a MB-aligned structure to the hypervisor.
>>
>>   [...]
>> **/
>>
>> #ifndef __BIOS_TABLES_TEST_H__
>> #define __BIOS_TABLES_TEST_H__
>>
>> [...]
>>
>> #endif // __BIOS_TABLES_TEST_H__
>
> It's possible that "scripts/clean-header-guards.pl" does not recognize
> the guard.

Correct.  I fixed the script in my tree.

> According to the ISO C standard, "All identifiers that begin with an
> underscore and either an uppercase letter or another underscore are
> always reserved for any use". Therefore, technically speaking, the above
> inclusion guard implies undefined behavior. In practice, this particular
> style for header guards is extremely common in the edk2 codebase:
>
> $ git grep '^#ifndef __' -- '*.h'  | wc -l
> 1012
>
> And, "tests/uefi-test-tools/UefiTestToolsPkg" follows the edk2 coding
> style.
>
> That said, if you'd like to remove the leading "__" from the macro name,
> I'd be fully OK with that.

We routinely exempt files from style cleanups when we have a reason.  If
you want this one to be exempted, that's fine with me.

If we decide not to exempt it, then I want a header guard that makes my
(fixed) script happy.  It isn't right now:

    $ scripts/clean-header-guards.pl -nv tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h 
    tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard __BIOS_TABLES_TEST_H__ needs cleanup:
        is a reserved identifier, doesn't end with _H, doesn't match the file name
    [...]

Removing the leading "__" takes care of the first complaint:

    tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H__ needs cleanup:
        doesn't end with _H, doesn't match the file name

Removing the trailing "__" as well takes care of the second one:

    tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H needs cleanup:
        doesn't match the file name

To get rid of the last one, we can:

* Rename to BIOSTABLESTEST_H.  Easy.

* Teach scripts/clean-header-guards.pl to capitalize StudlyCaps
  filenames to STUDLY_CAPS rather than STUDLYCAPS.  But that would break
  include/libdecnumber/*.h.

* Teach scripts/clean-header-guards to accept either STUDLYCAPS or
  STUDLY_CAPS.  Considering we have exactly one filename that needs
  this, I'd prefer not to.

My first preference is BIOSTABLESTEST_H, second is to exempt the file.
Yours?

Re: [Qemu-devel] Headers without multiple inclusion guards

[Philippe Mathieu-Daudé]

On 6/3/19 2:59 PM, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
>> Hi Markus,
>>
>> (sorry about the late reply, I've been away.)
>>
>> On 05/28/19 20:12, Markus Armbruster wrote:
>>
>>> EDK2 Firmware
>>> M: Laszlo Ersek <lersek@redhat.com>
>>> M: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
>>
>> This header file does have a multiple inclusion guard:
>>
>>> /** @file
>>>   Expose the address(es) of the ACPI RSD PTR table(s) and the SMBIOS entry
>>>   point(s) in a MB-aligned structure to the hypervisor.
>>>
>>>   [...]
>>> **/
>>>
>>> #ifndef __BIOS_TABLES_TEST_H__
>>> #define __BIOS_TABLES_TEST_H__
>>>
>>> [...]
>>>
>>> #endif // __BIOS_TABLES_TEST_H__
>>
>> It's possible that "scripts/clean-header-guards.pl" does not recognize
>> the guard.
> 
> Correct.  I fixed the script in my tree.
> 
>> According to the ISO C standard, "All identifiers that begin with an
>> underscore and either an uppercase letter or another underscore are
>> always reserved for any use". Therefore, technically speaking, the above
>> inclusion guard implies undefined behavior. In practice, this particular
>> style for header guards is extremely common in the edk2 codebase:
>>
>> $ git grep '^#ifndef __' -- '*.h'  | wc -l
>> 1012
>>
>> And, "tests/uefi-test-tools/UefiTestToolsPkg" follows the edk2 coding
>> style.
>>
>> That said, if you'd like to remove the leading "__" from the macro name,
>> I'd be fully OK with that.
> 
> We routinely exempt files from style cleanups when we have a reason.  If
> you want this one to be exempted, that's fine with me.
> 
> If we decide not to exempt it, then I want a header guard that makes my
> (fixed) script happy.  It isn't right now:
> 
>     $ scripts/clean-header-guards.pl -nv tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h 
>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard __BIOS_TABLES_TEST_H__ needs cleanup:
>         is a reserved identifier, doesn't end with _H, doesn't match the file name
>     [...]
> 
> Removing the leading "__" takes care of the first complaint:
> 
>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H__ needs cleanup:
>         doesn't end with _H, doesn't match the file name
> 
> Removing the trailing "__" as well takes care of the second one:
> 
>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H needs cleanup:
>         doesn't match the file name
> 
> To get rid of the last one, we can:
> 
> * Rename to BIOSTABLESTEST_H.  Easy.
> 
> * Teach scripts/clean-header-guards.pl to capitalize StudlyCaps
>   filenames to STUDLY_CAPS rather than STUDLYCAPS.  But that would break
>   include/libdecnumber/*.h.
> 
> * Teach scripts/clean-header-guards to accept either STUDLYCAPS or
>   STUDLY_CAPS.  Considering we have exactly one filename that needs
>   this, I'd prefer not to.
> 
> My first preference is BIOSTABLESTEST_H, second is to exempt the file.
> Yours?
> 

What about excluding UefiTestToolsPkg?

$ git grep '^#ifndef __' -- \
  '*.h' ':!tests/uefi-test-tools/UefiTestToolsPkg'

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laszlo Ersek]

On 06/03/19 16:24, Philippe Mathieu-Daudé wrote:
> On 6/3/19 2:59 PM, Markus Armbruster wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>>
>>> Hi Markus,
>>>
>>> (sorry about the late reply, I've been away.)
>>>
>>> On 05/28/19 20:12, Markus Armbruster wrote:
>>>
>>>> EDK2 Firmware
>>>> M: Laszlo Ersek <lersek@redhat.com>
>>>> M: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h
>>>
>>> This header file does have a multiple inclusion guard:
>>>
>>>> /** @file
>>>>   Expose the address(es) of the ACPI RSD PTR table(s) and the SMBIOS entry
>>>>   point(s) in a MB-aligned structure to the hypervisor.
>>>>
>>>>   [...]
>>>> **/
>>>>
>>>> #ifndef __BIOS_TABLES_TEST_H__
>>>> #define __BIOS_TABLES_TEST_H__
>>>>
>>>> [...]
>>>>
>>>> #endif // __BIOS_TABLES_TEST_H__
>>>
>>> It's possible that "scripts/clean-header-guards.pl" does not recognize
>>> the guard.
>>
>> Correct.  I fixed the script in my tree.
>>
>>> According to the ISO C standard, "All identifiers that begin with an
>>> underscore and either an uppercase letter or another underscore are
>>> always reserved for any use". Therefore, technically speaking, the above
>>> inclusion guard implies undefined behavior. In practice, this particular
>>> style for header guards is extremely common in the edk2 codebase:
>>>
>>> $ git grep '^#ifndef __' -- '*.h'  | wc -l
>>> 1012
>>>
>>> And, "tests/uefi-test-tools/UefiTestToolsPkg" follows the edk2 coding
>>> style.
>>>
>>> That said, if you'd like to remove the leading "__" from the macro name,
>>> I'd be fully OK with that.
>>
>> We routinely exempt files from style cleanups when we have a reason.  If
>> you want this one to be exempted, that's fine with me.
>>
>> If we decide not to exempt it, then I want a header guard that makes my
>> (fixed) script happy.  It isn't right now:
>>
>>     $ scripts/clean-header-guards.pl -nv tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h 
>>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard __BIOS_TABLES_TEST_H__ needs cleanup:
>>         is a reserved identifier, doesn't end with _H, doesn't match the file name
>>     [...]
>>
>> Removing the leading "__" takes care of the first complaint:
>>
>>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H__ needs cleanup:
>>         doesn't end with _H, doesn't match the file name
>>
>> Removing the trailing "__" as well takes care of the second one:
>>
>>     tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h guard BIOS_TABLES_TEST_H needs cleanup:
>>         doesn't match the file name
>>
>> To get rid of the last one, we can:
>>
>> * Rename to BIOSTABLESTEST_H.  Easy.
>>
>> * Teach scripts/clean-header-guards.pl to capitalize StudlyCaps
>>   filenames to STUDLY_CAPS rather than STUDLYCAPS.  But that would break
>>   include/libdecnumber/*.h.
>>
>> * Teach scripts/clean-header-guards to accept either STUDLYCAPS or
>>   STUDLY_CAPS.  Considering we have exactly one filename that needs
>>   this, I'd prefer not to.
>>
>> My first preference is BIOSTABLESTEST_H, second is to exempt the file.
>> Yours?
>>
> 
> What about excluding UefiTestToolsPkg?
> 
> $ git grep '^#ifndef __' -- \
>   '*.h' ':!tests/uefi-test-tools/UefiTestToolsPkg'
> 

Let's go with BIOSTABLESTEST_H for now. If UefiTestToolsPkg continues to
generate a bunch of warnings in other QEMU checkers too, we can still
decide to exclude it, later. This change looks small enough to me, for now.

BTW thanks for that nice git-grep syntax :) I guess I should read up on
"pathspec" in gitglossary(7) sometime...

Thanks!
Laszlo

Re: [Qemu-devel] Headers without multiple inclusion guards

[Daniel P. Berrangé]

On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
> 
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
> 
> Just in case: what's a multiple inclusion guard?  It's
> 
>     #ifndef UNIQUE_GUARD_SYMBOL_H
>     #define UNIQUE_GUARD_SYMBOL_H
>     ...
>     #endif
> 
> with nothing but comments outside the conditional, so that the header
> can safely be included more than once.

Any opinions on using the less verbose syntax instead:

  #pragma once

It is not portable C, but we explicitly only care about GCC or CLang,
so portability isn't an issue for us.


> Cryptography
> M: Daniel P. Berrange <berrange@redhat.com>
> crypto/ivgen-essiv.h
> crypto/ivgen-plain.h
> crypto/ivgen-plain64.h

These have header guards present

> tests/crypto-tls-psk-helpers.h
> tests/crypto-tls-x509-helpers.h

These should be fixed.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laszlo Ersek]

On 06/05/19 16:23, Daniel P. Berrangé wrote:
> On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
>> We have a bunch of headers without multiple inclusion guards.  Some are
>> clearly intentional, some look accidental.  Too many for me to find out
>> by examining each of them, so I'm asking their maintainers.
>>
>> Why do I ask?  I'd like to mark the intentional ones and fix the
>> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
>> 0/7] Baby steps towards saner headers" just because they lack multiple
>> inclusion guards.
>>
>> Just in case: what's a multiple inclusion guard?  It's
>>
>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>     #define UNIQUE_GUARD_SYMBOL_H
>>     ...
>>     #endif
>>
>> with nothing but comments outside the conditional, so that the header
>> can safely be included more than once.
> 
> Any opinions on using the less verbose syntax instead:
> 
>   #pragma once
> 
> It is not portable C, but we explicitly only care about GCC or CLang,
> so portability isn't an issue for us.

I don't feel strongly about this particular question, so if folks prefer
"#pragma once", I won't complain. I'd just like to report that section 6
of "HACKING" appears the opposite of "we explicitly only care about GCC
or CLang".

Thanks,
Laszlo

> 
> 
>> Cryptography
>> M: Daniel P. Berrange <berrange@redhat.com>
>> crypto/ivgen-essiv.h
>> crypto/ivgen-plain.h
>> crypto/ivgen-plain64.h
> 
> These have header guards present
> 
>> tests/crypto-tls-psk-helpers.h
>> tests/crypto-tls-x509-helpers.h
> 
> These should be fixed.
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] Headers without multiple inclusion guards

[Daniel P. Berrangé]

On Wed, Jun 05, 2019 at 06:18:38PM +0200, Laszlo Ersek wrote:
> On 06/05/19 16:23, Daniel P. Berrangé wrote:
> > On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> >> We have a bunch of headers without multiple inclusion guards.  Some are
> >> clearly intentional, some look accidental.  Too many for me to find out
> >> by examining each of them, so I'm asking their maintainers.
> >>
> >> Why do I ask?  I'd like to mark the intentional ones and fix the
> >> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> >> 0/7] Baby steps towards saner headers" just because they lack multiple
> >> inclusion guards.
> >>
> >> Just in case: what's a multiple inclusion guard?  It's
> >>
> >>     #ifndef UNIQUE_GUARD_SYMBOL_H
> >>     #define UNIQUE_GUARD_SYMBOL_H
> >>     ...
> >>     #endif
> >>
> >> with nothing but comments outside the conditional, so that the header
> >> can safely be included more than once.
> > 
> > Any opinions on using the less verbose syntax instead:
> > 
> >   #pragma once
> > 
> > It is not portable C, but we explicitly only care about GCC or CLang,
> > so portability isn't an issue for us.
> 
> I don't feel strongly about this particular question, so if folks prefer
> "#pragma once", I won't complain. I'd just like to report that section 6
> of "HACKING" appears the opposite of "we explicitly only care about GCC
> or CLang".

Heh, yeah that doc does require some updating, as we certainly don't
require conformance to the vanilla C standard any more.  We now explicitly
request "-std=gnu99" as the conformance level which allows many GNU
extensions to the standard.

commit 7be41675f7cb16be7c8d2554add7a63fa43781a8
Author: Thomas Huth <thuth@redhat.com>
Date:   Mon Jan 7 11:25:22 2019 +0100

    configure: Force the C standard to gnu99
    
    Different versions of GCC and Clang use different versions of the C standard.
    This repeatedly caused problems already, e.g. with duplicated typedefs:
    
     https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
    
    or with for-loop variable initializers:
    
     https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
    
    To avoid these problems, we should enforce the C language version to the
    same level for all compilers. Since our minimum compiler versions is
    GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
    available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
    for the few C++ code that we have in the repository.


The HACKING docs predate this commit, and when reviewing, we failed to
notice we needed to update HACKING to match our new policy.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Headers without multiple inclusion guards

[Laszlo Ersek]

On 06/05/19 18:23, Daniel P. Berrangé wrote:
> On Wed, Jun 05, 2019 at 06:18:38PM +0200, Laszlo Ersek wrote:
>> On 06/05/19 16:23, Daniel P. Berrangé wrote:
>>> On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
>>>> We have a bunch of headers without multiple inclusion guards.  Some are
>>>> clearly intentional, some look accidental.  Too many for me to find out
>>>> by examining each of them, so I'm asking their maintainers.
>>>>
>>>> Why do I ask?  I'd like to mark the intentional ones and fix the
>>>> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
>>>> 0/7] Baby steps towards saner headers" just because they lack multiple
>>>> inclusion guards.
>>>>
>>>> Just in case: what's a multiple inclusion guard?  It's
>>>>
>>>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>>>     #define UNIQUE_GUARD_SYMBOL_H
>>>>     ...
>>>>     #endif
>>>>
>>>> with nothing but comments outside the conditional, so that the header
>>>> can safely be included more than once.
>>>
>>> Any opinions on using the less verbose syntax instead:
>>>
>>>   #pragma once
>>>
>>> It is not portable C, but we explicitly only care about GCC or CLang,
>>> so portability isn't an issue for us.
>>
>> I don't feel strongly about this particular question, so if folks prefer
>> "#pragma once", I won't complain. I'd just like to report that section 6
>> of "HACKING" appears the opposite of "we explicitly only care about GCC
>> or CLang".
> 
> Heh, yeah that doc does require some updating, as we certainly don't
> require conformance to the vanilla C standard any more.  We now explicitly
> request "-std=gnu99" as the conformance level which allows many GNU
> extensions to the standard.
> 
> commit 7be41675f7cb16be7c8d2554add7a63fa43781a8
> Author: Thomas Huth <thuth@redhat.com>
> Date:   Mon Jan 7 11:25:22 2019 +0100
> 
>     configure: Force the C standard to gnu99
>     
>     Different versions of GCC and Clang use different versions of the C standard.
>     This repeatedly caused problems already, e.g. with duplicated typedefs:
>     
>      https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
>     
>     or with for-loop variable initializers:
>     
>      https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
>     
>     To avoid these problems, we should enforce the C language version to the
>     same level for all compilers. Since our minimum compiler versions is
>     GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
>     available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
>     for the few C++ code that we have in the repository.

TIL :)

Thanks!
Laszlo

> The HACKING docs predate this commit, and when reviewing, we failed to
> notice we needed to update HACKING to match our new policy.
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
>> We have a bunch of headers without multiple inclusion guards.  Some are
>> clearly intentional, some look accidental.  Too many for me to find out
>> by examining each of them, so I'm asking their maintainers.
>> 
>> Why do I ask?  I'd like to mark the intentional ones and fix the
>> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
>> 0/7] Baby steps towards saner headers" just because they lack multiple
>> inclusion guards.
>> 
>> Just in case: what's a multiple inclusion guard?  It's
>> 
>>     #ifndef UNIQUE_GUARD_SYMBOL_H
>>     #define UNIQUE_GUARD_SYMBOL_H
>>     ...
>>     #endif
>> 
>> with nothing but comments outside the conditional, so that the header
>> can safely be included more than once.
>
> Any opinions on using the less verbose syntax instead:
>
>   #pragma once
>
> It is not portable C, but we explicitly only care about GCC or CLang,
> so portability isn't an issue for us.

I doubt its worth the churn.  But I'm content to go with the flow here.

>> Cryptography
>> M: Daniel P. Berrange <berrange@redhat.com>
>> crypto/ivgen-essiv.h
>> crypto/ivgen-plain.h
>> crypto/ivgen-plain64.h
>
> These have header guards present

Yes.  They #include outside the header guard, which confuses my script.

>> tests/crypto-tls-psk-helpers.h
>> tests/crypto-tls-x509-helpers.h
>
> These should be fixed.

Will do, thanks!

Re: [Qemu-devel] Headers without multiple inclusion guards

[Alistair Francis]

On Tue, May 28, 2019 at 11:13 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> We have a bunch of headers without multiple inclusion guards.  Some are
> clearly intentional, some look accidental.  Too many for me to find out
> by examining each of them, so I'm asking their maintainers.
>
> Why do I ask?  I'd like to mark the intentional ones and fix the
> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> 0/7] Baby steps towards saner headers" just because they lack multiple
> inclusion guards.
>

Sorry for the delay I have been on holidays. I think the ARM ones have
all been covered by Peter, here is some info on the RISC-V includes.

....

> linux-user/riscv/sockbits.h
> linux-user/riscv/syscall_nr.h
> linux-user/riscv/target_syscall.h
> linux-user/riscv/termbits.h

I'm not sure here, it looks like all targets have the same guards, so
just follow what the others do here.

....

> target/riscv/cpu_bits.h
> target/riscv/cpu_user.h

These should have guards

> target/riscv/helper.h

Intentional

> target/riscv/instmap.h

I think this should have a guard.

Alistair

Re: [Qemu-devel] Headers without multiple inclusion guards

[Daniel P. Berrangé]

On Wed, Jun 05, 2019 at 07:52:50PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, May 28, 2019 at 08:12:24PM +0200, Markus Armbruster wrote:
> >> We have a bunch of headers without multiple inclusion guards.  Some are
> >> clearly intentional, some look accidental.  Too many for me to find out
> >> by examining each of them, so I'm asking their maintainers.
> >> 
> >> Why do I ask?  I'd like to mark the intentional ones and fix the
> >> accidental ones, so they don't flunk "make check-headers" from "[RFC v4
> >> 0/7] Baby steps towards saner headers" just because they lack multiple
> >> inclusion guards.
> >> 
> >> Just in case: what's a multiple inclusion guard?  It's
> >> 
> >>     #ifndef UNIQUE_GUARD_SYMBOL_H
> >>     #define UNIQUE_GUARD_SYMBOL_H
> >>     ...
> >>     #endif
> >> 
> >> with nothing but comments outside the conditional, so that the header
> >> can safely be included more than once.
> >
> > Any opinions on using the less verbose syntax instead:
> >
> >   #pragma once
> >
> > It is not portable C, but we explicitly only care about GCC or CLang,
> > so portability isn't an issue for us.
> 
> I doubt its worth the churn.  But I'm content to go with the flow here.

Since the collective response was effectively "Meh", lets just
pretend i didn't raise this suggestion :-)


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|