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.camu...@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 <jad...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel