[PATCH weston v2 5/6] Add API to retrieve and iterate over the resources list of a client
Jonas Ådahl
jadahl at gmail.com
Tue Aug 9 08:17:02 UTC 2016
On Tue, Jul 05, 2016 at 09:51:10AM +0200, Giulio Camuffo wrote:
> To complement on the new resource created signal, this allows to
> iterate over the existing resources of a client.
>
> Signed-off-by: Giulio Camuffo <giulio.camuffo at kdab.com>
I know the break:ability of the iteration was discussed in the previous
version. I can't think of a situation where it'll ever break. If you
need to iterate through all the resources to get a particular one, you
are probably doing something wrong. It also makes the API a bit more
confusing, since a numeric return value that effects a for-each loop
will always be hard to follow.
FWIW, glib tries to solve a similiar API issue by trying to make people
return G_SOURCE_CONTINUE and G_SOURCE_REMOVE for a similar situation,
because returning FALSE/TRUE made reading the code annoying, but without
actually using a enum or something like that forcing people to write
more readable code, it's almost pointless.
With that said, the code looks correct to me, and with a couple of
comments inline, this is:
Reviewed-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>
> v2: - don't cast the function pointer, but make it a two step process
> - make it possible to break the loop early
>
> src/wayland-private.h | 2 +-
> src/wayland-server-core.h | 8 ++++++++
> src/wayland-server.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/wayland-util.c | 8 ++++++--
> 4 files changed, 61 insertions(+), 4 deletions(-)
>
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index 045109b..b715534 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -74,7 +74,7 @@ struct wl_map {
> uint32_t free_list;
> };
>
> -typedef void (*wl_iterator_func_t)(void *element, void *data);
> +typedef int (*wl_iterator_func_t)(void *element, void *data);
Not sure if this matters, but this changes the type signature of an
exported function symbol: wl_map_for_each. While WL_EXPORT:ed, it's not
exposed via any installed header file, so I suppose it won't cause any
real problems.
>
> void
> wl_map_init(struct wl_map *map, uint32_t side);
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index bb0a989..647d97c 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -216,6 +216,14 @@ void
> wl_client_add_resource_created_listener(struct wl_client *client,
> struct wl_listener *listener);
>
> +typedef int (*wl_client_for_each_resource_iterator_func_t)(struct wl_resource *,
nit: missing parameter name
> + void *user_data);
> +
> +void
> +wl_client_for_each_resource(struct wl_client *client,
> + wl_client_for_each_resource_iterator_func_t iterator,
> + void *user_data);
> +
> /** \class wl_listener
> *
> * \brief A single listener for Wayland signals
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 067d8a5..98e3bda 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -565,7 +565,7 @@ wl_resource_post_no_memory(struct wl_resource *resource)
> WL_DISPLAY_ERROR_NO_MEMORY, "no memory");
> }
>
> -static void
> +static int
> destroy_resource(void *element, void *data)
> {
> struct wl_resource *resource = element;
> @@ -580,6 +580,8 @@ destroy_resource(void *element, void *data)
>
> if (!(flags & WL_MAP_ENTRY_LEGACY))
> free(resource);
> +
> + return 0;
> }
>
> WL_EXPORT void
> @@ -1603,6 +1605,49 @@ wl_client_add_resource_created_listener(struct wl_client *client,
> wl_signal_add(&client->resource_created_signal, listener);
> }
>
> +struct wl_resource_iterator_context {
> + void *user_data;
> + wl_client_for_each_resource_iterator_func_t it;
> +};
> +
> +static int
> +resource_iterator_helper(void *res, void *user_data)
> +{
> + struct wl_resource_iterator_context *context = user_data;
> + struct wl_resource *resource = res;
> + return context->it(resource, context->user_data);
> +}
> +
> +/** Iterate over all the resources of a client
> + *
> + * \param client The client object
> + * \param iterator The iterator function
> + * \param user_data The user data pointer
> + *
> + * The function pointed by \a iterator will be called for each
> + * resource owned by the client. The \a user_data will be passed
> + * as the second argument of the iterator function.
> + * If the \a iterator function returns a value less than 0 the loop
> + * will stop.
nit: "... the iteration will stop."
Jonas
> + *
> + * Creating and destroying resources while iterating is safe, but new
> + * resources may or may not be picked up by the iterator.
> + *
> + * \memberof wl_client
> + */
> +WL_EXPORT void
> +wl_client_for_each_resource(struct wl_client *client,
> + wl_client_for_each_resource_iterator_func_t iterator,
> + void *user_data)
> +{
> + struct wl_resource_iterator_context context = {
> + .user_data = user_data,
> + .it = iterator,
> + };
> +
> + wl_map_for_each(&client->objects, resource_iterator_helper, &context);
> +}
> +
> /** \cond */ /* Deprecated functions below. */
>
> uint32_t
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index 7467366..856e53b 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -363,13 +363,17 @@ static void
> for_each_helper(struct wl_array *entries, wl_iterator_func_t func, void *data)
> {
> union map_entry *start, *end, *p;
> + int ret;
>
> start = entries->data;
> end = (union map_entry *) ((char *) entries->data + entries->size);
>
> for (p = start; p < end; p++)
> - if (p->data && !map_entry_is_free(*p))
> - func(map_entry_get_data(*p), data);
> + if (p->data && !map_entry_is_free(*p)) {
> + ret = func(map_entry_get_data(*p), data);
> + if (ret < 0)
> + break;
> + }
> }
>
> WL_EXPORT void
> --
> 2.9.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list