Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On Mon, 2016-01-25 at 14:49 +, Andrew Cooper wrote:
> On 25/01/16 14:47, Ian Campbell wrote:
> > On Mon, 2016-01-25 at 14:35 +, Ian Jackson wrote:
> > > Ian Campbell writes ("Re: [Xen-devel] [PATCH XEN v8 02/29] tools:
> > > Refactor /dev/xen/evtchn wrappers into libxenevtchn."):
> > > > Various of the tools/libs/*/include/*.h have a
> > > >
> > > > /* Callers who don't care don't need to #include
> > > > */
> > > > typedef struct xentoollog_logger xentoollog_logger;
> > > >
> > > > but since that typedef matches in all cases I think it is allowed
> > > > to be
> > > > repeated like this, isn't it?
> > > No, I'm afraid not.
> > :-/
> >
> > > It is permitted to repeatedly mention `struct
> > > xentoollog_logger', but the typedef must only occur once in any
> > > translation unit.
> > For some reason I thought the typedef's could be repeated so long as
> > they
> > were identical.
> >
> > Seems at least Debian's gcc tolerates such things (which makes me
> > wonder
> > how many more instances of this might have snuck in, I guess folks like
> > Boris would have spotted them pretty quick).
> >
> > > If it is desirable to let callers avoid including xentoollog.h, then
> > > all those headers need to say:
> > >
> > > struct xentoollog_logger;
> > > int some_function(..., struct xentoollog_logger *lg, ...);
> > >
> > > So in your patches something like:
> > >
> > > struct xentoollog_logger;
> > >
> > > xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
> > > unsigned open_flags);
> > I'll do something like this for tools/libs/*/include.
> >
> > FYI the underlying patches are all in since Friday.
>
> How about doing the header strictness check on all headers at one? If
> repeating typedefs is a gccism, that should catch it.
AFAICT the issue here is that newer gcc tolerates duplicate typedefs under
many circumstances. Seems to take -pedantic-errors to make it not, and I
don't see another to enable the warning.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On 25/01/16 14:47, Ian Campbell wrote:
> On Mon, 2016-01-25 at 14:35 +, Ian Jackson wrote:
>> Ian Campbell writes ("Re: [Xen-devel] [PATCH XEN v8 02/29] tools:
>> Refactor /dev/xen/evtchn wrappers into libxenevtchn."):
>>> Various of the tools/libs/*/include/*.h have a
>>>
>>> /* Callers who don't care don't need to #include */
>>> typedef struct xentoollog_logger xentoollog_logger;
>>>
>>> but since that typedef matches in all cases I think it is allowed to be
>>> repeated like this, isn't it?
>> No, I'm afraid not.
> :-/
>
>> It is permitted to repeatedly mention `struct
>> xentoollog_logger', but the typedef must only occur once in any
>> translation unit.
> For some reason I thought the typedef's could be repeated so long as they
> were identical.
>
> Seems at least Debian's gcc tolerates such things (which makes me wonder
> how many more instances of this might have snuck in, I guess folks like
> Boris would have spotted them pretty quick).
>
>> If it is desirable to let callers avoid including xentoollog.h, then
>> all those headers need to say:
>>
>> struct xentoollog_logger;
>> int some_function(..., struct xentoollog_logger *lg, ...);
>>
>> So in your patches something like:
>>
>> struct xentoollog_logger;
>>
>> xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
>>unsigned open_flags);
> I'll do something like this for tools/libs/*/include.
>
> FYI the underlying patches are all in since Friday.
How about doing the header strictness check on all headers at one? If
repeating typedefs is a gccism, that should catch it.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On Mon, 2016-01-25 at 14:35 +, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH XEN v8 02/29] tools:
> Refactor /dev/xen/evtchn wrappers into libxenevtchn."):
> > Various of the tools/libs/*/include/*.h have a
> >
> > /* Callers who don't care don't need to #include */
> > typedef struct xentoollog_logger xentoollog_logger;
> >
> > but since that typedef matches in all cases I think it is allowed to be
> > repeated like this, isn't it?
>
> No, I'm afraid not.
:-/
> It is permitted to repeatedly mention `struct
> xentoollog_logger', but the typedef must only occur once in any
> translation unit.
For some reason I thought the typedef's could be repeated so long as they
were identical.
Seems at least Debian's gcc tolerates such things (which makes me wonder
how many more instances of this might have snuck in, I guess folks like
Boris would have spotted them pretty quick).
> If it is desirable to let callers avoid including xentoollog.h, then
> all those headers need to say:
>
> struct xentoollog_logger;
> int some_function(..., struct xentoollog_logger *lg, ...);
>
> So in your patches something like:
>
> struct xentoollog_logger;
>
> xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
> unsigned open_flags);
I'll do something like this for tools/libs/*/include.
FYI the underlying patches are all in since Friday.
>
> The separate `struct xentoollog_logger;' is needed because otherwise
> the `struct xentoollog_logger *logger' in the formal parameters of
> xenevtchn_open is a declaration, rather than a reference to a
> previously-declared thing, and if it is a declaration its scope is
> only the contained function prototype, so other mentions of `struct
> xentoollog_logger' are treated as references to a different type.
> This is, of course, insane.
>
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On 01/25/2016 09:35 AM, Ian Jackson wrote:
Ian Campbell writes ("Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor
/dev/xen/evtchn wrappers into libxenevtchn."):
Various of the tools/libs/*/include/*.h have a
/* Callers who don't care don't need to #include */
typedef struct xentoollog_logger xentoollog_logger;
but since that typedef matches in all cases I think it is allowed to be
repeated like this, isn't it?
No, I'm afraid not. It is permitted to repeatedly mention `struct
xentoollog_logger', but the typedef must only occur once in any
translation unit.
If it is desirable to let callers avoid including xentoollog.h, then
all those headers need to say:
struct xentoollog_logger;
int some_function(..., struct xentoollog_logger *lg, ...);
So in your patches something like:
struct xentoollog_logger;
xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
unsigned open_flags);
The separate `struct xentoollog_logger;' is needed because otherwise
the `struct xentoollog_logger *logger' in the formal parameters of
xenevtchn_open is a declaration, rather than a reference to a
previously-declared thing, and if it is a declaration its scope is
only the contained function prototype, so other mentions of `struct
xentoollog_logger' are treated as references to a different type.
This is, of course, insane.
(I was just about to respond to IanC's earlier message)
It builds on my desktop (gcc 4.9.2) but not on an older setup (4.4.7).
BTW, I should have explicitly said so: I haven't bisected to this commit
--- it just looked like this ould be the culprit.
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
Ian Campbell writes ("Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor
/dev/xen/evtchn wrappers into libxenevtchn."):
> Various of the tools/libs/*/include/*.h have a
>
> /* Callers who don't care don't need to #include */
> typedef struct xentoollog_logger xentoollog_logger;
>
> but since that typedef matches in all cases I think it is allowed to be
> repeated like this, isn't it?
No, I'm afraid not. It is permitted to repeatedly mention `struct
xentoollog_logger', but the typedef must only occur once in any
translation unit.
If it is desirable to let callers avoid including xentoollog.h, then
all those headers need to say:
struct xentoollog_logger;
int some_function(..., struct xentoollog_logger *lg, ...);
So in your patches something like:
struct xentoollog_logger;
xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
unsigned open_flags);
The separate `struct xentoollog_logger;' is needed because otherwise
the `struct xentoollog_logger *logger' in the formal parameters of
xenevtchn_open is a declaration, rather than a reference to a
previously-declared thing, and if it is a declaration its scope is
only the contained function prototype, so other mentions of `struct
xentoollog_logger' are treated as references to a different type.
This is, of course, insane.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On Fri, 2016-01-22 at 12:12 -0500, Boris Ostrovsky wrote: > On 01/15/2016 08:22 AM, Ian Campbell wrote: > > libxenevtchn will provide a stable API and ABI for accessing the > > evtchn device. > > I think this patch breaks the build: > > root@ovs101> gcc -O1 -fno-omit-frame-pointer -m64 -g > -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes > -Wdeclaration-after-statement -Wno-unused-but-set-variable -O0 -g3 > -D__XEN_TOOLS__ -MMD -MF .core.o.d -D_LARGEFILE_SOURCE > -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -Werror > -Wmissing-prototypes -I./include > -I/root/xen/tools/libs/evtchn/../../../tools/include > -I/root/xen/tools/libs/evtchn/../../../tools/libs/toollog/include > -I/root/xen/tools/libs/evtchn/../../../tools/include -c -o core.o core.c > In file included from private.h:5, > from core.c:19: > ./include/xenevtchn.h:36: error: redefinition of typedef ‘xentoollog_logger’ > /root/xen/tools/libs/evtchn/../../../tools/libs/toollog/include/xentoollog.h:44: > > note: previous declaration of ‘xentoollog_logger’ was here Hrm, it seems to be OK by the end of the series, in that it built for me before push and has passed smoke-test. So I may have introduced a bisection hazard (sorry) but it's too late to fix that unfortunately. Various of the tools/libs/*/include/*.h have a /* Callers who don't care don't need to #include */ typedef struct xentoollog_logger xentoollog_logger; but since that typedef matches in all cases I think it is allowed to be repeated like this, isn't it? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH XEN v8 02/29] tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.
On 01/15/2016 08:22 AM, Ian Campbell wrote: libxenevtchn will provide a stable API and ABI for accessing the evtchn device. I think this patch breaks the build: root@ovs101> gcc -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .core.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -Werror -Wmissing-prototypes -I./include -I/root/xen/tools/libs/evtchn/../../../tools/include -I/root/xen/tools/libs/evtchn/../../../tools/libs/toollog/include -I/root/xen/tools/libs/evtchn/../../../tools/include -c -o core.o core.c In file included from private.h:5, from core.c:19: ./include/xenevtchn.h:36: error: redefinition of typedef ‘xentoollog_logger’ /root/xen/tools/libs/evtchn/../../../tools/libs/toollog/include/xentoollog.h:44: note: previous declaration of ‘xentoollog_logger’ was here -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
