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

[Pekka Paalanen]

On Tue, 19 Feb 2013 16:09:21 -0600
Jason Ekstrand <jason at jlekstrand.net> wrote:

> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
> This version contains the documentation changes as requested by Pekka Paalanen
> as well as the if-statement change suggested by Bill Spitzak.
> 
>  src/connection.c        | 15 +++++++++----
>  src/wayland-client.c    | 13 +++++++++--
>  src/wayland-private.h   |  3 ++-
>  src/wayland-server.c    |  9 +++++++-
>  src/wayland-util.h      | 35 +++++++++++++++++++++++++++--
>  tests/dispatcher-test.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+), 10 deletions(-)
> 
...
> diff --git a/src/wayland-util.h b/src/wayland-util.h
> index df7b384..2d3367f 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -46,18 +46,49 @@ 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 we always check the version number before
> + * reading any fields that are not in the original structure, we should be
> + * fine.
> + *
> + * Special care must, however, be taken with the auto-generated instances of
> + * wl_interface created by the scanner.  Because each of the generated
> + * interfaces is a global extern variable, the memory for them may be allocated
> + * by the client-code and not by the library.  Because the client code
> + * allocates only allocates enough memory for the size it knows about, any
> + * fields unknown to the client cannot be trusted. Therefore, increasing the
> + * interface version of wl_interface globals generated by the scanner
> + * constitutes an ABI break.

Alright, this looks better. An extra 'allocates'.
Don't you mean struct version for the ABI break?

This does make me wonder, is it possible for the global to be allocated
in one size, and then initialized according to a bigger size, leading
to overwriting some memory? I mean at runtime when linking a library,
in case globals are copied. Does that even happen? The dynamic linker
won't look at the struct version. My gut is saying that this must be
already handled properly and there's no need to worry, but I've been
surprised before.

I also have hard time understanding the API break. If we have an
application built with struct version 0, and libwayland built with
struct version 1, does that break?

Especially considering the case, if the application has a generated
*-protocol.c file of its own protocol extension, which will then
reference core interface globals like wl_surface_interface.

Maybe we are saved by the fact that all the globals are const? Wait...
struct wl_interfaces is not a const anymore, since you need to be able
to assign the dispatcher function pointers at runtime, do you not?

Btw. shouldn't you modify wayland-scanner to not make them const then?

Could you explain or demonstrate how you actually plug in your custom
dispatchers? I couldn't find any hint of that in the v3 patch set,
did I just miss it?

Ah, but is the whole point, that you will re-generate your own set of
all wl_interface structs with your own scanner, which will plug in the
right dispatcher functions, and everything can still be const?

> + */
>  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;
>  };


Thanks,
pq