[PATCH wayland] scanner: server-side also emit SINCE macros for requests

[Pekka Paalanen]

On Wed, 14 Oct 2015 17:05:43 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Wed, Oct 14, 2015 at 11:52:36AM +0300, Pekka Paalanen wrote:
> > On Wed, 14 Oct 2015 10:32:36 +0300
> > Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> > 
> > > 2015-10-14 4:06 GMT+03:00 Jonas Ådahl <jadahl at gmail.com>:
> > > > On Tue, Oct 13, 2015 at 04:18:19PM -0700, Bryce Harrington wrote:
> > > >> On Fri, Oct 09, 2015 at 10:07:48PM +0200, David FORT wrote:
> > > >> > Server-side we need to know the versions for destructor requests.
> > > >> >
> > > >> > Signed-off-by: David FORT <contact at hardening-consulting.com>
> > > >>
> > > >> Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
> > > >>
> > > >> > ---
> > > >> >  src/scanner.c | 1 +
> > > >> >  1 file changed, 1 insertion(+)
> > > >> >
> > > >> > diff --git a/src/scanner.c b/src/scanner.c
> > > >> > index f456aa5..406ba82 100644
> > > >> > --- a/src/scanner.c
> > > >> > +++ b/src/scanner.c
> > > >> > @@ -1266,6 +1266,7 @@ emit_header(struct protocol *protocol, enum side side)
> > > >> >                     emit_structs(&i->request_list, i, side);
> > > >> >                     emit_opcodes(&i->event_list, i);
> > > >> >                     emit_opcode_versions(&i->event_list, i);
> > > >> > +                   emit_opcode_versions(&i->request_list, i);
> > > >
> > > > This will cause compilation error when an event and a request in the
> > > > same interface has the same name. Do we have that restriction already?
> > > 
> > > That sounds real confusing. If we don't, we should have it, imho.
> > 
> > We kind of sort of may have it...
> > 
> > See this made up xml:
> > 
> > <protocol name="dupe">
> > 
> >   <interface name="stupid" version="2">
> >     <request name="dupp" />
> >     <event name="dummy">
> >       <description summary="bump the opcode" />
> >     </event>
> >     <event name="dupp" since="2" />
> >   </interface>
> > 
> > </protocol>
> > 
> > This will already conflict without any "since" #definitions if both
> > client and server header are included in the same compilation unit,
> > because server header has
> > 
> > #define STUPID_DUPP	1
> > 
> > and client header has
> > 
> > #define STUPID_DUPP	0
> > 
> > This is a far-fetched example, but I think it shows that we really
> > should not have requests and events of the same name.
> > 
> > I would propose the following:
> > 
> > 1. Patch wayland-scanner to reject protocols where an interface has
> >    both a request and an event with the same name.
> > 2. Wait for a stable Wayland release.
> > 3. Start relying on never to have a request and an event with the same name.
> 
> Do you mean we should revert the commit that causes the scanner to
> generate
> 
> #define STUPID_DUPP	1
> 
> until after the next release? Or should we assume these hypothetical
> protocols don't act as both clients and servers in the same file?o

We can't revert, it was already there since before 0.85. :-)
This is the opcode #def, not the "since".

I mean we should hold off on this thread's patch before we are sure
enough we don't get into trouble for it.

> Also, if we make a release where we reject such protocols, then we could
> just as well start rely on it already (at build time). If it'd be just a
> warning, we can wait with relying on it when we make that warning an
> error.

It could be a warning, sure. But it doesn't matter if it's an error or
a warning, if it gets hit by a stable protocol extension.

I'm only saying that it is much easier to revert the rejection, than to
remove things from generated API, if it turns out we seriously broke
something.

> Relying on it at runtime; I don't see in what scenario we would need to
> care about this.

I never talked about runtime. If you apply this patch, you said we will
rely on never having the same name in both requests and events (if they
have different "since"). Otherwise the build will fail.

> > This is overly cautious, but I think it is possible people have got
> > away with using a same name, and we should know if anyone wants to
> > complain while we have a chance to back out.

Or, if you are confident that alpha and beta releases get wide enough
testing, then we can do everything in one go.

The worst situation that I am trying to test for and avoid is that we
have a stable Wayland release which exposes the SINCE macros and while
doing that it breaks the build of a third-party protocol extension that
is already stable. In that situation it will be difficult to remove or
change the SINCE macros since they are already in the stable API.

The request vs. event conflict is a guaranteed one, while the server
vs. client headers conflict we can have today is very unlikely to
trigger in well-organized code bases (separate complication units for
server vs. client side).


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151014/a78d96ad/attachment.sig>