[PATCH wayland 4/7] Add support for custom dispatchers

[Jason Ekstrand]

On Tue, Feb 19, 2013 at 2:22 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Mon, 18 Feb 2013 10:03:25 -0600
> Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>>
>> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
>> ---
>>  src/connection.c        | 15 +++++++++----
>>  src/wayland-client.c    | 13 +++++++++--
>>  src/wayland-private.h   |  3 ++-
>>  src/wayland-server.c    |  9 +++++++-
>>  src/wayland-util.h      | 27 ++++++++++++++++++++--
>>  tests/dispatcher-test.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 117 insertions(+), 10 deletions(-)
>>
> ...
>> diff --git a/src/wayland-util.h b/src/wayland-util.h
>> index df7b384..b77a122 100644
>> --- a/src/wayland-util.h
>> +++ b/src/wayland-util.h
>> @@ -46,18 +46,41 @@ struct wl_message {
>>       const struct wl_interface **types;
>>  };
>>
>> +union wl_argument;
>> +struct wl_object;
>> +
>> +typedef void (*wl_interface_dispatcher_func_t)(struct wl_object *, uint32_t,
>> +                                            const struct wl_message *,
>> +                                            void *, union wl_argument *);
>> +/**
>> + * wl_interface - object method interface
>> + *
>> + * This structure has been extended since libwayland version 1.  The version
>> + * field now represents both an interface version and a structure version.
>> + * The upper 16 bits represent the version of the structure with the original
>> + * version being version 0.  The lower 16 bits represent the interface version
>> + * as specified in the interface XML file.
>> + *
>> + * This way, as long as the version number is something sane and instances of
>> + * "struct wl_interface" are never statically allocated, older software should
>> + * still complie and link against newer versions of libwayland.
>> + */
>>  struct wl_interface {
>>       const char *name;
>> -     int version;
>> +     uint32_t version;
>>       int method_count;
>>       const struct wl_message *methods;
>>       int event_count;
>>       const struct wl_message *events;
>> +
>> +     /* Added in version 1 */
>> +     wl_interface_dispatcher_func_t method_dispatcher;
>> +     wl_interface_dispatcher_func_t event_dispatcher;
>>  };
>>
>>  struct wl_object {
>>       const struct wl_interface *interface;
>> -     void (* const * implementation)(void);
>> +     const void *implementation;
>>       uint32_t id;
>>  };
>
> Hi Jason,
>
> the comment above makes me a little worried. We do have struct
> wl_interface statically allocated in both server and client code in the
> weston repository, for example:
> - clients/workspaces-protocol.c
> - src/workspaces-protocol.c
>
> These are parts of protocol extensions, that are not provided
> by libwayland. The above files are generated by wayland-scanner, of
> course.
>
> Doesn't the above change break such programs, that are compiled with
> old libwayland and then ran with new libwayland?
>
> Well, I don't think it breaks, because we never allocate or use arrays
> of struct wl_interface, right? We only have arrays of pointers to struct
> wl_interface, and on every use of a wl_interface you check the version,
> so we do not access non-existant fields.
>
> So maybe the comment just needs a little adjustment on how wl_interface
> can be allocated. Also, "complie". :-)
>
>
> Thanks,
> pq

Pekka,
Thanks for pointing that out.  It is, indeed, merely a documentation
error.  I wrote that comment before I had a chance to play with the
linking issues involved in the process.  Instances of wl_interface can
be statically allocated without causing trouble.  The main thing is
that we only pass pointers into libwayland functions and that we
always check the version field before checking anything not in the
original interface.

There is one other issue that gets sticky and should be better
documented.  When dynamically linking the extern variables, they
apparently get allocated as per the client code and not the library.
In short, this means the scanner cannot generate an interface version
greater than what the client knows about.
--Jason