"Michael S. Tsirkin" <m...@redhat.com> writes: > On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: >> On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: >> > docs/devel/style.rst mandates: >> > >> > The "qemu/osdep.h" header contains preprocessor macros that affect >> > the behavior of core system headers like <stdint.h>. It must be >> > the first include so that core system headers included by external >> > libraries get the preprocessor macros that QEMU depends on. >> > >> > Do not include "qemu/osdep.h" from header files since the .c file >> > will have already included it. >> > >> > A few violations have crept in. Fix them. >> > >> > Signed-off-by: Markus Armbruster <arm...@redhat.com> >> > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> >> > Reviewed-by: Bin Meng <bmeng...@gmail.com> >> > Reviewed-by: Taylor Simpson <tsimp...@quicinc.com> >> > Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> >> >> With my awesome grep skillz I found one more: >> $ grep -r --include='*.h' qemu/osdep.h >> include/block/graph-lock.h:#include "qemu/osdep.h"
Crept in after I prepared my v1. I neglected to re-check. > Also: > $ grep -r --include='*.inc' qemu/osdep.h > ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" > crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" > crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" > crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" > crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" > target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include > "qemu/osdep.h" > target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/cris/translate_v10.c.inc:#include "qemu/osdep.h" Good point. Looks like I successfully supressed all memory of .inc. >> Looks like all C files must include qemu/osdep.h, no? I remember there are a few exceptions, but I don't remember which .c they are. Hmm... see commit 4bd802b209cff612d1a99674a91895b735be8630. >> How about >> >> 1- add -include qemu/osdep.h on compile command line >> drop #include "qemu/osdep.h" from C files Then you need to encode the exceptions in the build system. Which might not be a bad thing. >> 2- drop double include guards, replace with a warning. >> >> following patch implements part 2: >> >> >> qemu/osdep: don't include it from headers >> >> doing so will lead to trouble eventually - instead of >> working around such cases make it more likely it will fail. >> >> Signed-off-by: Michael S. Tsirkin <m...@redhat.com> >> >> --- >> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h >> index 7d059ad526..e4a60f911c 100644 >> --- a/include/qemu/osdep.h >> +++ b/include/qemu/osdep.h >> @@ -24,7 +24,12 @@ >> * This work is licensed under the terms of the GNU GPL, version 2 or later. >> * See the COPYING file in the top-level directory. >> */ >> -#ifndef QEMU_OSDEP_H >> +#ifdef QEMU_OSDEP_H >> +#warning "Never include qemu/osdep.h from a header!" >> +#endif >> + >> +static inline void qemu_osdep_never_include_from_header(void) {} >> + Why do you need the function, too? >> #define QEMU_OSDEP_H >> >> #include "config-host.h" >> @@ -714,5 +719,3 @@ static inline int platform_does_not_support_system(const >> char *command) >> #ifdef __cplusplus >> } >> #endif >> - >> -#endif