Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc

[Alexey Kardashevskiy]

On 09/03/17 01:21, Peter Maydell wrote:
> On 8 March 2017 at 13:49, Alexey Kardashevskiy <address@hidden> wrote:
>> After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
>> compiling in CentOS7:
>>
>> In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
>>                  from /home/aik/p/qemu/device_tree.c:30:
>> /home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined 
>> [-Werror]
>>  #define __bitwise
>>  ^
>> In file included from /usr/include/asm/ptrace.h:27:0,
>>                  from /usr/include/asm/sigcontext.h:11,
>>                  from /usr/include/bits/sigcontext.h:27,
>>                  from /usr/include/signal.h:340,
>>                  from /home/aik/p/qemu/include/qemu/osdep.h:86,
>>                  from /home/aik/p/qemu/device_tree.c:14:
>> /usr/include/linux/types.h:21:0: note: this is the location of the previous 
>> definition
>>  #define __bitwise __bitwise__
>>  ^
>> cc1: all warnings being treated as errors
>> make: *** [device_tree.o] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>> The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
>> the internal one which does not compile as CentOS7 comes with gcc v4.8.5
>> which reports warnings which it would not if the OS's libfdt was used
>> (libfdt_env.h has not changed between 1.4.0 and 1.4.2).
>>
>> gcc 6.2.0 from Ubuntu v16.10 handles this fine.
>>
>> This replaces -I with -isystem to suppress the warning (which turns
>> to an error because of -Werror).
> 
> Thanks for the bug report. I think it would be cleaner to fix this

Agree.

> by fixing the problem upstream in libfdt and then moving our
> submodule forward to the fixed version. (libfdt should not be
> defining __ prefixed symbols as these are reserved for the
> system.)

Also agree, regardless my proposal, libfdt should be fixed. However v2.9
won't compile on CentOS7 (and I suppose on RHEL7) which is a bit annoying.
And having "-isystem" makes some sense as these headers are normally system
ones and I'd think they should be treated the same way (i.e. path should be
included via -isystem vs. -I).



-- 
Alexey