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

[Markus Armbruster <armbru@redhat.com>]

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!