| Message ID | 1521645545-247351-2-git-send-email-mst@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <sw@weilnetz.de> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- Reviewed-by: Daniel P. Berrangé <berrange@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
On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote: > 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. It's borderline. In particular there is at least one instance where we already have a conflict (yes, there are probably other ways to address this, but I consider them less robust): util/getauxval.c:#include <sys/auxv.h> /usr/include/sys/auxv.h:#include <elf.h> So it really does build by luck. > > 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 :-( Judging by git log output: For clang it appears to be there since 2007 that's pretty close to day 1. gcc's log is better documented, it's been there since 2004-05-03 > > > > As a side effect, this catches any future uses of > > #include <> for internal headers. > > > > Suggested-by: Stefan Weil <sw@weilnetz.de> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Thanks! > > > > > 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 :|
On 21/03/2018 18:02, Michael S. Tsirkin wrote: > On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote: >> 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. > > It's borderline. > > In particular there is at least one instance where we already have a > conflict (yes, there are probably other ways to address this, but > I consider them less robust): > > util/getauxval.c:#include <sys/auxv.h> > /usr/include/sys/auxv.h:#include <elf.h> > > So it really does build by luck. > > >>> 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 :-( > > Judging by git log output: > > For clang it appears to be there since 2007 that's pretty > close to day 1. > > gcc's log is better documented, it's been there since 2004-05-03 > >>> >>> As a side effect, this catches any future uses of >>> #include <> for internal headers. >>> >>> Suggested-by: Stefan Weil <sw@weilnetz.de> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >>> --- >> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > Thanks! > Hi Michael, Should I add this patch to the RDMA pull request? Thanks, Marcel >> >>> >>> 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 :|
On Wed, Mar 21, 2018 at 06:40:23PM +0200, Marcel Apfelbaum wrote: > On 21/03/2018 18:02, Michael S. Tsirkin wrote: > > On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote: > >> 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. > > > > It's borderline. > > > > In particular there is at least one instance where we already have a > > conflict (yes, there are probably other ways to address this, but > > I consider them less robust): > > > > util/getauxval.c:#include <sys/auxv.h> > > /usr/include/sys/auxv.h:#include <elf.h> > > > > So it really does build by luck. > > > > > >>> 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 :-( > > > > Judging by git log output: > > > > For clang it appears to be there since 2007 that's pretty > > close to day 1. > > > > gcc's log is better documented, it's been there since 2004-05-03 > > > >>> > >>> As a side effect, this catches any future uses of > >>> #include <> for internal headers. > >>> > >>> Suggested-by: Stefan Weil <sw@weilnetz.de> > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > >>> --- > >> > >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > > > > Thanks! > > > > Hi Michael, > Should I add this patch to the RDMA pull request? > > Thanks, > Marcel Go ahead and add both of them pls. > > >> > >>> > >>> 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 :|
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. > > To fix, switch to -iquote which is supported by both > gcc and clang and only affects #include "" directives. > > As a side effect, this catches any future uses of > #include <> for internal headers. > > Suggested-by: Stefan Weil <sw@weilnetz.de> > Signed-off-by: Michael S. Tsirkin <mst@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(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
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
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. To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives. As a side effect, this catches any future uses of #include <> for internal headers. Suggested-by: Stefan Weil <sw@weilnetz.de> Signed-off-by: Michael S. Tsirkin <mst@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(-)