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

[Jonas Ådahl]

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

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.

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


Jonas

> 
> 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.
> 
> 
> Thanks,
> pq