[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote |
Date: |
Wed, 21 Mar 2018 18:40:23 +0200 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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 <address@hidden>
>>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>>> ---
>>
>> Reviewed-by: Daniel P. Berrangé <address@hidden>
>
>
> 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
>> :|
Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives, Daniel P . Berrangé, 2018/03/21
Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives, Marcel Apfelbaum, 2018/03/21
Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives, Yuval Shaia, 2018/03/21