[PATCH] wl_resource: Add version field and getter/setter

[Jason Ekstrand]

Alex,
Really, this looks like a pretty clean solution.  I've spent a fair amount
of time banging around trying to extend wl_resource and this is better than
anything I ever came up with.  I only have the one comment (already
mentioned on IRC but repeated here for the sake of the list).

On Thu, May 23, 2013 at 1:28 PM, <alexl at redhat.com> wrote:

> From: Alexander Larsson <alexl at redhat.com>
>
> We create a private structure for extra data and store it in
> a destroy notifier. In this way we can store the version
> in a backwards compatible way.
>
> This lets us track the actual version of a resource which
> is generally the min of what the client requested and what
> the server supports. This will let us avoid sending messages
> the client doesn't support and to not handle requests the
> server doesn't support.
> ---
>  src/wayland-server.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/wayland-server.h |  6 ++++++
>  2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index a3d3887..c808f6a 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -102,6 +102,11 @@ struct wl_global {
>         struct wl_list link;
>  };
>
> +struct wl_resource_private {
> +       struct wl_listener listener;
> +       uint32_t version;
> +};
> +
>  static int wl_debug = 0;
>
>  static void
> @@ -377,6 +382,14 @@ WL_EXPORT uint32_t
>  wl_client_add_resource(struct wl_client *client,
>                        struct wl_resource *resource)
>  {
> +       struct wl_resource_private *priv;
> +
> +       priv = calloc(1, sizeof(struct wl_resource_private));
> +       if (!priv) {
> +               wl_resource_post_no_memory(resource);
> +               return 0;
> +       }
> +
>         if (resource->object.id == 0) {
>                 resource->object.id =
>                         wl_map_insert_new(&client->objects,
> @@ -387,12 +400,18 @@ wl_client_add_resource(struct wl_client *client,
>                                        WL_DISPLAY_ERROR_INVALID_OBJECT,
>                                        "invalid new id %d",
>                                        resource->object.id);
> +               free(priv);
>                 return 0;
>         }
>
>         resource->client = client;
>         wl_signal_init(&resource->destroy_signal);
>
> +       priv->listener.notify = (wl_notify_func_t)free;
> +       wl_signal_add(&resource->destroy_signal, &priv->listener);
> +
> +       priv->version = 1;
> +
>         return resource->object.id;
>  }
>
> @@ -895,6 +914,35 @@ wl_display_get_destroy_listener(struct wl_display
> *display,
>         return wl_signal_get(&display->destroy_signal, notify);
>  }
>
> +static struct wl_resource_private *
> +wl_resource_get_priv (struct wl_resource *resource)
> +{
> +       /* The priv pointer is always the notify handler */
> +       return (struct wl_resource_private
> *)resource->destroy_signal.listener_list.next;
> +}
>

wl_signal_get might be better here.  It's more robust in (somewhat odd)
case the user code decides to manually put a listener at the beginning.
It's also more readable and almost just as fast (different by an if).


> +
> +WL_EXPORT void
> +wl_resource_set_version(struct wl_resource *resource,
> +                       uint32_t version)
> +{
> +       struct wl_resource_private *priv = wl_resource_get_priv (resource);
> +
> +       priv->version = version;
> +}
> +
> +WL_EXPORT uint32_t
> +wl_resource_get_version(struct wl_resource *resource)
> +{
> +       struct wl_resource_private *priv = wl_resource_get_priv (resource);
> +
> +       return priv->version;
> +}
> +
> +static void
> +dummy_notify(struct wl_listener *listener, void *data)
> +{
> +}
> +
>  WL_EXPORT struct wl_resource *
>  wl_client_add_object(struct wl_client *client,
>                      const struct wl_interface *interface,
> @@ -902,8 +950,9 @@ wl_client_add_object(struct wl_client *client,
>                      uint32_t id, void *data)
>  {
>         struct wl_resource *resource;
> +       struct wl_resource_private *priv;
>
> -       resource = malloc(sizeof *resource);
> +       resource = malloc(sizeof *resource + sizeof (struct
> wl_resource_private));
>         if (resource == NULL) {
>                 wl_resource_post_no_memory(client->display_resource);
>                 return NULL;
> @@ -913,6 +962,13 @@ wl_client_add_object(struct wl_client *client,
>         resource->client = client;
>         resource->destroy = (void *) free;
>
> +       priv = (struct wl_resource_private *)(resource + 1);
> +
> +       priv->listener.notify = dummy_notify;
> +       wl_signal_add(&resource->destroy_signal, &priv->listener);
> +
> +       priv->version = 1;
> +
>         if (wl_map_insert_at(&client->objects, resource->object.id,
> resource) < 0) {
>                 wl_resource_post_error(client->display_resource,
>                                        WL_DISPLAY_ERROR_INVALID_OBJECT,
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index a9cf544..6beb0dd 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -135,6 +135,12 @@ wl_client_new_object(struct wl_client *client,
>  struct wl_resource *
>  wl_client_get_object(struct wl_client *client, uint32_t id);
>
> +void
> +wl_resource_set_version(struct wl_resource *resource,
> +                       uint32_t version);
> +uint32_t
> +wl_resource_get_version(struct wl_resource *resource);
> +
>  struct wl_listener {
>         struct wl_list link;
>         wl_notify_func_t notify;
> --
> 1.8.1.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>

Thanks,
--Jason Ekstrand
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130523/0b2743c8/attachment.html>