Re: [Xen-devel] Compile errors with gcc 4.8

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.13 at 10:06, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2013-02-07 at 08:59 +0000, M A Young wrote:
>> On Thu, 7 Feb 2013, Andrew Cooper wrote:
>> 
>> > On 06/02/2013 20:37, M A Young wrote:
>> >
>> >> There are two types of problem, the first is from
>> >> xen/common/compat/memory.c where the error is
>> >>
>> >> memory.c: In function 'compat_memory_op':
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33:
>> >> error: typedef '__guest_handle_const_compat_memory_exchange_t' locally
>> >> defined but not used [-Werror=unused-local-typedefs]
>> >>       typedef struct { type *p; } __guest_handle_ ## name
>> >>                                   ^
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5:
>> >> note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
>> >>       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>> >>       ^
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41:
>> >> note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
>> >>   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name,
>> >> name)
>> >>                                           ^
>> >> memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
>> >>               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
>> >>               ^
>> >>
>> >> so we are defining something that isn't used.
>> >
>> > But it is used, by guest_handle_cast, albeit through several layers of
>> > macros.
>> >
>> > I would tentativly object to the use of "__attribute__((unused));" to
>> > work around what appears to be a gcc bug.  If 4.8 support is desperately
>> > wanted, then a better solution would be to specify
>> > -Wno-unused-local-typedefs to disable the buggy feature.
>> 
>> It looked to me as if the DEFINE macro does two typedefs and the code only 
>> uses the first, so gcc 4.8 was correct in its objections. The suggested 
>> workaround of using -Wno-unused-local-typedefs isn't very portible as 
>> earlier versions of gcc don't recognize it.
> 
> We have a cc-option macro in the makefiles to enable these sorts of
> options to be enabled conditionally.
> 
> However it seems a bit odd to be using DEFINE_XEN_GUEST_HANDLE in this
> manner, should it not be declared in some header? I can see a
> DEFINE_COMPAT_HANDLE(compat_memory_exchange_t) in
> xen/include/compat/memory.h. Perhaps the one in memory.c can just be
> dropped or otherwise modified to use this?

Actually, I'm of the opposite opinion - it should be in other places
too that handles don't get needlessly defined by the public
headers. They should get defined only when there actually is a
use for them. Everything else can be defined where actually
consumed, as in this case: For one, a handle of a compat_*
type can't be defined in a public header anyway, as the compat
types don't get defined there. And then, as you say, the oddity
of this type makes it desirable to scope restrict it as much as
possible.

Now, for the actual solution, I'd prefer the -Wno-... option
suggested above, or as a second best choice generalizing the
solution suggested by Michael, applying the attribute in
DEFINE_XEN_GUEST_HANDLE() itself. This is the more that
attaching it in to the use of the macro just happens to work,
but isn't guaranteed to in the future: Switching around the
order of the two lines of the expansion

#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
    ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
    ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)

or adding a third (say, volatile) one would re-expose the
problem.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel