* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
@ 2019-05-28 18:32 ` Peter Maydell
2019-05-29 12:47 ` Markus Armbruster
2019-05-28 19:23 ` Eduardo Habkost
` (16 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2019-05-28 18:32 UTC (permalink / raw)
To: Markus Armbruster
Cc: Chris Wulff, Sagar Karandikar, David Hildenbrand, James Hogan,
Anthony Green, Palmer Dabbelt, Mark Cave-Ayland, QEMU Developers,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Richard Henderson, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Riku Voipio, Eduardo Habkost, Alistair Francis,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:32 ` Peter Maydell
@ 2019-05-29 12:47 ` Markus Armbruster
2019-05-30 10:14 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2019-05-29 12:47 UTC (permalink / raw)
To: Peter Maydell
Cc: Dmitry Fleytman, Sagar Karandikar, Michael S. Tsirkin,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
QEMU Developers, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Markus Armbruster, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Daniel P. Berrange, Claudio Fontana,
Bastian Koppelmann, Chris Wulff, Claudio Fontana, Laurent Vivier,
Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 12:47 ` Markus Armbruster
@ 2019-05-30 10:14 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2019-05-30 10:14 UTC (permalink / raw)
To: Markus Armbruster
Cc: Dmitry Fleytman, Sagar Karandikar, Michael S. Tsirkin,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
QEMU Developers, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Daniel P. Berrange, Claudio Fontana,
Bastian Koppelmann, Chris Wulff, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
2019-05-28 18:32 ` Peter Maydell
@ 2019-05-28 19:23 ` Eduardo Habkost
2019-05-29 8:21 ` Paul Durrant
2019-05-28 19:55 ` Max Filippov
` (15 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Eduardo Habkost @ 2019-05-28 19:23 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Alistair Francis, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, David Gibson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Cornelia Huck,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 19:23 ` Eduardo Habkost
@ 2019-05-29 8:21 ` Paul Durrant
2019-05-29 12:49 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Paul Durrant @ 2019-05-29 8:21 UTC (permalink / raw)
To: 'Eduardo Habkost', Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Alistair Francis, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Stafford Horne, Alex Bennée,
David Gibson, Daniel P. Berrange, Claudio Fontana,
Bastian Koppelmann, Cornelia Huck, Claudio Fontana,
Laurent Vivier, Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
> [...]
> > 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 8:21 ` Paul Durrant
@ 2019-05-29 12:49 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-05-29 12:49 UTC (permalink / raw)
To: Paul Durrant
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, 'Eduardo Habkost',
Riku Voipio, Greg Kurz, Marcelo Tosatti, Beniamino Galvani,
Eric Auger, Stafford Horne, Alex Bennée, Richard Henderson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
2019-05-28 18:32 ` Peter Maydell
2019-05-28 19:23 ` Eduardo Habkost
@ 2019-05-28 19:55 ` Max Filippov
2019-05-29 12:51 ` Markus Armbruster
2019-05-28 20:08 ` Richard Henderson
` (14 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Max Filippov @ 2019-05-28 19:55 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Richard Henderson, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Riku Voipio, Eduardo Habkost, Alistair Francis,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 19:55 ` Max Filippov
@ 2019-05-29 12:51 ` Markus Armbruster
2019-05-29 13:20 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2019-05-29 12:51 UTC (permalink / raw)
To: Max Filippov
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao,
Marek Vasut, Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
David Hildenbrand, Halil Pasic, Christian Borntraeger,
Anthony Perard, David Gibson, Laszlo Ersek, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Alistair Francis,
Eduardo Habkost, Riku Voipio, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 12:51 ` Markus Armbruster
@ 2019-05-29 13:20 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-29 13:20 UTC (permalink / raw)
To: Markus Armbruster, Max Filippov
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao,
Marek Vasut, Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
David Hildenbrand, Halil Pasic, Christian Borntraeger,
Anthony Perard, David Gibson, Laszlo Ersek, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Alistair Francis,
Eduardo Habkost, Riku Voipio, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann, Michael Walle,
Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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/.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (2 preceding siblings ...)
2019-05-28 19:55 ` Max Filippov
@ 2019-05-28 20:08 ` Richard Henderson
2019-05-28 22:14 ` BALATON Zoltan
` (13 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Richard Henderson @ 2019-05-28 20:08 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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~
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (3 preceding siblings ...)
2019-05-28 20:08 ` Richard Henderson
@ 2019-05-28 22:14 ` BALATON Zoltan
2019-05-29 1:47 ` David Gibson
` (12 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2019-05-28 22:14 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Richard Henderson, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Riku Voipio, Eduardo Habkost, Alistair Francis,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (4 preceding siblings ...)
2019-05-28 22:14 ` BALATON Zoltan
@ 2019-05-29 1:47 ` David Gibson
2019-05-29 6:30 ` Laurent Vivier
` (11 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-05-29 1:47 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Riku Voipio,
Eduardo Habkost, Alistair Francis, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Cornelia Huck,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
[-- 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 --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (5 preceding siblings ...)
2019-05-29 1:47 ` David Gibson
@ 2019-05-29 6:30 ` Laurent Vivier
2019-05-29 8:25 ` Bastian Koppelmann
` (10 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Laurent Vivier @ 2019-05-29 6:30 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Richard Henderson, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Riku Voipio, Eduardo Habkost, Alistair Francis,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Philippe Mathieu-Daudé,
Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (6 preceding siblings ...)
2019-05-29 6:30 ` Laurent Vivier
@ 2019-05-29 8:25 ` Bastian Koppelmann
2019-05-29 9:01 ` Cornelia Huck
` (9 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Bastian Koppelmann @ 2019-05-29 8:25 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Daniel P. Berrange, Claudio Fontana,
Philippe Mathieu-Daudé,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
On 5/28/19 8:12 PM, Markus Armbruster wrote:
> target/tricore/tricore-opcodes.h
unintentional.
Cheers,
Bastian
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (7 preceding siblings ...)
2019-05-29 8:25 ` Bastian Koppelmann
@ 2019-05-29 9:01 ` Cornelia Huck
2019-05-29 9:19 ` Greg Kurz
` (8 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Cornelia Huck @ 2019-05-29 9:01 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
David Hildenbrand, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Max Filippov, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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'...]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (8 preceding siblings ...)
2019-05-29 9:01 ` Cornelia Huck
@ 2019-05-29 9:19 ` Greg Kurz
2019-05-29 9:38 ` Alex Bennée
` (7 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2019-05-29 9:19 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, David Gibson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Cornelia Huck,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (9 preceding siblings ...)
2019-05-29 9:19 ` Greg Kurz
@ 2019-05-29 9:38 ` Alex Bennée
2019-05-29 9:58 ` Anthony PERARD
` (6 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2019-05-29 9:38 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Cornelia Huck, Sagar Karandikar,
David Hildenbrand, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Max Filippov, Dmitry Fleytman,
Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao,
Marek Vasut, Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, David Gibson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (10 preceding siblings ...)
2019-05-29 9:38 ` Alex Bennée
@ 2019-05-29 9:58 ` Anthony PERARD
2019-05-29 12:55 ` Markus Armbruster
2019-05-29 10:05 ` David Hildenbrand
` (5 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Anthony PERARD @ 2019-05-29 9:58 UTC (permalink / raw)
To: Markus Armbruster, Stefano Stabellini; +Cc: Paul Durrant, qemu-devel
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 9:58 ` Anthony PERARD
@ 2019-05-29 12:55 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2019-05-29 12:55 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Stefano Stabellini, qemu-devel, Paul Durrant
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (11 preceding siblings ...)
2019-05-29 9:58 ` Anthony PERARD
@ 2019-05-29 10:05 ` David Hildenbrand
2019-05-29 13:22 ` Philippe Mathieu-Daudé
` (4 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: David Hildenbrand @ 2019-05-29 10:05 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, Michael S. Tsirkin,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
>
> S390
> M: Richard Henderson <rth@twiddle.net>
> M: David Hildenbrand <david@redhat.com>
> target/s390x/helper.h
Intentional.
Cheers!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (12 preceding siblings ...)
2019-05-29 10:05 ` David Hildenbrand
@ 2019-05-29 13:22 ` Philippe Mathieu-Daudé
2019-05-29 14:10 ` Markus Armbruster
2019-06-02 7:06 ` Dmitry Fleytman
` (3 subsequent siblings)
17 siblings, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-29 13:22 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin,
Laszlo Ersek, Halil Pasic, Christian Borntraeger, Anthony Perard,
Richard Henderson, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Riku Voipio, Eduardo Habkost, Alistair Francis,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, 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__
...
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 13:22 ` Philippe Mathieu-Daudé
@ 2019-05-29 14:10 ` Markus Armbruster
2019-06-03 10:57 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2019-05-29 14:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Daniel P. Berrange, Claudio Fontana,
Bastian Koppelmann, Chris Wulff, Claudio Fontana, Laurent Vivier,
Andrew Baumann, Michael Walle, Aleksandar Markovic,
Cornelia Huck, Paolo Bonzini, Aurelien Jarno
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-29 14:10 ` Markus Armbruster
@ 2019-06-03 10:57 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2019-06-03 10:57 UTC (permalink / raw)
To: Markus Armbruster, Philippe Mathieu-Daudé
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Alistair Francis,
Eduardo Habkost, Riku Voipio, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann, Michael Walle,
Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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.)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (13 preceding siblings ...)
2019-05-29 13:22 ` Philippe Mathieu-Daudé
@ 2019-06-02 7:06 ` Dmitry Fleytman
2019-06-03 10:55 ` Laszlo Ersek
` (2 subsequent siblings)
17 siblings, 0 replies; 36+ messages in thread
From: Dmitry Fleytman @ 2019-06-02 7:06 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
>
> hw/net/e1000e_core.h
> hw/net/e1000x_common.h
> hw/net/vmxnet3_defs.h
Unintentional.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (14 preceding siblings ...)
2019-06-02 7:06 ` Dmitry Fleytman
@ 2019-06-03 10:55 ` Laszlo Ersek
2019-06-03 12:59 ` Markus Armbruster
2019-06-05 14:23 ` Daniel P. Berrangé
2019-06-05 20:49 ` Alistair Francis
17 siblings, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2019-06-03 10:55 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
Max Filippov, Dmitry Fleytman, Alistair Francis, Gerd Hoffmann,
Edgar E. Iglesias, Guan Xuetao, Marek Vasut, Stefano Stabellini,
Aleksandar Rikalo, Helge Deller, Michael S. Tsirkin, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Cornelia Huck, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-03 10:55 ` Laszlo Ersek
@ 2019-06-03 12:59 ` Markus Armbruster
2019-06-03 14:24 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2019-06-03 12:59 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Philippe Mathieu-Daudé,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Alistair Francis, Eduardo Habkost, Riku Voipio, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, Richard Henderson,
Daniel P. Berrange, Claudio Fontana, Bastian Koppelmann,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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?
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-03 12:59 ` Markus Armbruster
@ 2019-06-03 14:24 ` Philippe Mathieu-Daudé
2019-06-03 16:23 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-03 14:24 UTC (permalink / raw)
To: Markus Armbruster, Laszlo Ersek
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Alistair Francis,
Eduardo Habkost, Riku Voipio, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann, Michael Walle,
Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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'
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-03 14:24 ` Philippe Mathieu-Daudé
@ 2019-06-03 16:23 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2019-06-03 16:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Markus Armbruster
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson, Jason Wang,
Artyom Tarasenko, Laurent Vivier, Thomas Huth, Alistair Francis,
Eduardo Habkost, Riku Voipio, Greg Kurz, Marcelo Tosatti,
Beniamino Galvani, Eric Auger, Paul Durrant, Stafford Horne,
Alex Bennée, Richard Henderson, Daniel P. Berrange,
Claudio Fontana, Bastian Koppelmann, Chris Wulff,
Claudio Fontana, Laurent Vivier, Andrew Baumann, Michael Walle,
Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (15 preceding siblings ...)
2019-06-03 10:55 ` Laszlo Ersek
@ 2019-06-05 14:23 ` Daniel P. Berrangé
2019-06-05 16:18 ` Laszlo Ersek
2019-06-05 17:52 ` Markus Armbruster
2019-06-05 20:49 ` Alistair Francis
17 siblings, 2 replies; 36+ messages in thread
From: Daniel P. Berrangé @ 2019-06-05 14:23 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Laszlo Ersek, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson, Claudio Fontana,
Bastian Koppelmann, Cornelia Huck, Claudio Fontana,
Laurent Vivier, Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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 :|
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-05 14:23 ` Daniel P. Berrangé
@ 2019-06-05 16:18 ` Laszlo Ersek
2019-06-05 16:23 ` Daniel P. Berrangé
2019-06-05 17:52 ` Markus Armbruster
1 sibling, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2019-06-05 16:18 UTC (permalink / raw)
To: Daniel P. Berrangé, Markus Armbruster
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Halil Pasic, Christian Borntraeger,
Anthony Perard, Richard Henderson, Jason Wang, Artyom Tarasenko,
Laurent Vivier, Thomas Huth, Riku Voipio, Eduardo Habkost,
Alistair Francis, Greg Kurz, Marcelo Tosatti, Beniamino Galvani,
Eric Auger, Paul Durrant, Stafford Horne, Alex Bennée,
David Gibson, Claudio Fontana, Bastian Koppelmann, Cornelia Huck,
Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-05 16:18 ` Laszlo Ersek
@ 2019-06-05 16:23 ` Daniel P. Berrangé
2019-06-05 17:03 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Daniel P. Berrangé @ 2019-06-05 16:23 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Markus Armbruster, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson, Claudio Fontana,
Bastian Koppelmann, Cornelia Huck, Claudio Fontana,
Laurent Vivier, Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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 :|
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-05 16:23 ` Daniel P. Berrangé
@ 2019-06-05 17:03 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2019-06-05 17:03 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, Chris Wulff, Sagar Karandikar, David Hildenbrand,
James Hogan, Anthony Green, Palmer Dabbelt, Mark Cave-Ayland,
qemu-devel, Max Filippov, Dmitry Fleytman, Alistair Francis,
Gerd Hoffmann, Edgar E. Iglesias, Guan Xuetao, Marek Vasut,
Stefano Stabellini, Aleksandar Rikalo, Helge Deller,
Michael S. Tsirkin, Markus Armbruster, Halil Pasic,
Christian Borntraeger, Anthony Perard, Richard Henderson,
Jason Wang, Artyom Tarasenko, Laurent Vivier, Thomas Huth,
Riku Voipio, Eduardo Habkost, Alistair Francis, Greg Kurz,
Marcelo Tosatti, Beniamino Galvani, Eric Auger, Paul Durrant,
Stafford Horne, Alex Bennée, David Gibson, Claudio Fontana,
Bastian Koppelmann, Cornelia Huck, Claudio Fontana,
Laurent Vivier, Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Paolo Bonzini,
Aurelien Jarno, Philippe Mathieu-Daudé
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-05 14:23 ` Daniel P. Berrangé
2019-06-05 16:18 ` Laszlo Ersek
@ 2019-06-05 17:52 ` Markus Armbruster
2019-06-07 17:32 ` Daniel P. Berrangé
1 sibling, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2019-06-05 17:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Claudio Fontana, Bastian Koppelmann,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-06-05 17:52 ` Markus Armbruster
@ 2019-06-07 17:32 ` Daniel P. Berrangé
0 siblings, 0 replies; 36+ messages in thread
From: Daniel P. Berrangé @ 2019-06-07 17:32 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel, Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Claudio Fontana, Bastian Koppelmann,
Chris Wulff, Claudio Fontana, Laurent Vivier, Andrew Baumann,
Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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 :|
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] Headers without multiple inclusion guards
2019-05-28 18:12 [Qemu-devel] Headers without multiple inclusion guards Markus Armbruster
` (16 preceding siblings ...)
2019-06-05 14:23 ` Daniel P. Berrangé
@ 2019-06-05 20:49 ` Alistair Francis
17 siblings, 0 replies; 36+ messages in thread
From: Alistair Francis @ 2019-06-05 20:49 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, Dmitry Fleytman, Sagar Karandikar,
Michael S. Tsirkin, James Hogan, Anthony Green, Palmer Dabbelt,
Mark Cave-Ayland, qemu-devel@nongnu.org Developers,
Philippe Mathieu-Daudé,
Max Filippov, Alistair Francis, Gerd Hoffmann, Edgar E. Iglesias,
Guan Xuetao, Marek Vasut, Stefano Stabellini, Aleksandar Rikalo,
Helge Deller, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Anthony Perard, David Gibson,
Laszlo Ersek, Jason Wang, Artyom Tarasenko, Laurent Vivier,
Thomas Huth, Alistair Francis, Eduardo Habkost, Riku Voipio,
Greg Kurz, Marcelo Tosatti, Beniamino Galvani, Eric Auger,
Paul Durrant, Stafford Horne, Alex Bennée,
Richard Henderson, Daniel P. Berrange, Claudio Fontana,
Bastian Koppelmann, Chris Wulff, Claudio Fontana, Laurent Vivier,
Andrew Baumann, Philippe Mathieu-Daudé,
Michael Walle, Aleksandar Markovic, Cornelia Huck, Paolo Bonzini,
Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread