On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote: > Our rule right now is to use <> for external headers, > "" for internal ones. The idea was to avoid conflicts > between e.g. a system file named <trace.h> and an > internal one by the same name. > > Unfortunately we use -I compiler flag so it does not > help: a system file doing #include <trace.h> will > still pick up ours first.
Has that actually hit you practice ? I'm wondering if this is just theoretical in which case it could wait till 2.13 since this scenario has existed in QEMU along time, or a real problem right now requiring fix in 2.12. > To fix, switch to -iquote which is supported by both > gcc and clang and only affects #include "" directives. Fine since we don't support anything other than gcc and clang. I'm assuming -iquote has been supported by these two compilers for a long time though ? The GCC docs annoyingly don't ever mention what release features appear in :-( > > As a side effect, this catches any future uses of > #include <> for internal headers. > > Suggested-by: Stefan Weil <s...@weilnetz.de> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > --- Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > > I still think we want to switch to a more formal rule such as qemu/ > prefix for all includes down the road, but this will at least catch any > scheme violations from creeping in meanwhile. > > > configure | 16 ++++++++-------- > rules.mak | 2 +- > Makefile.target | 4 ++-- > 3 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/configure b/configure > index af72fc8..23a4f3b 100755 > --- a/configure > +++ b/configure > @@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv > $QEMU_CFLAGS" > QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" > QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" > QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > $QEMU_CFLAGS" > -QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg > -I\$(SRC_PATH)/include" > +QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg > -iquote \$(SRC_PATH)/include" > if test "$debug_info" = "yes"; then > CFLAGS="-g $CFLAGS" > LDFLAGS="-g $LDFLAGS" > @@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then > fi > > if test "$tcg_interpreter" = "yes"; then > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES" > elif test "$ARCH" = "sparc64" ; then > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES" > elif test "$ARCH" = "s390x" ; then > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES" > elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES" > elif test "$ARCH" = "ppc64" ; then > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES" > else > - QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES" > + QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES" > fi > -QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES" > +QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES" > > echo "TOOLS=$tools" >> $config_host_mak > echo "ROMS=$roms" >> $config_host_mak > diff --git a/rules.mak b/rules.mak > index 6e94333..93a0702 100644 > --- a/rules.mak > +++ b/rules.mak > @@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d > # dir, one absolute and the other relative to the compiler working > # directory. These are the same for target-independent files, but > # different for target-dependent ones. > -QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D) > +QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D) > > WL_U := -Wl,-u, > find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2))) > diff --git a/Makefile.target b/Makefile.target > index 6549481..d0ec77a 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR)) > ifdef CONFIG_LINUX > QEMU_CFLAGS += -I../linux-headers > endif > -QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H > +QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) > -DNEED_CPU_H > > -QEMU_CFLAGS+=-I$(SRC_PATH)/include > +QEMU_CFLAGS+=-iquote $(SRC_PATH)/include > > ifdef CONFIG_USER_ONLY > # user emulator name > -- > MST > 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 :|