[PATCH] Add a new wl_proxy_set_listener function
Pekka Paalanen
ppaalanen at gmail.com
Fri Jun 17 10:38:25 UTC 2016
On Sat, 19 Mar 2016 18:14:35 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> The wl_proxy_add_listener function is ill-named, because it suggests
> that a proxy can have many listeners. Instead it can only have one so
> deprecate the old function and add a new wl_proxy_set_listener. The
> implementation of the new function is exactly the same implementation
> wl_proxy_add_listener had, and that is now implemented by calling
> wl_proxy_set_listener.
> Also make the scanner output new <interface>_set_listener functions.
Hi,
a good idea.
Missing S-o-b. I suppose this is the latest patch? It doesn't say v2.
I marked the other one as superseded.
This patch has an easy to solve conflict with the proxy wrappers patch,
and now there is also this:
tests/queue-test.c: In function ‘client_test_queue_proxy_wrapper’:
tests/queue-test.c:242:2: warning: ‘wl_callback_add_listener’ is deprecated (declared at ./protocol/wayland-client-protocol.h:1123) [-Wdeprecated-declarations]
wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
^
tests/queue-test.c: In function ‘client_test_queue_set_queue_race’:
tests/queue-test.c:289:2: warning: ‘wl_callback_add_listener’ is deprecated (declared at ./protocol/wayland-client-protocol.h:1123) [-Wdeprecated-declarations]
wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
^
> ---
> src/scanner.c | 20 ++++++++++++++++++--
> src/wayland-client-core.h | 12 +++++++++++-
> src/wayland-client.c | 23 +++++++++++++++++++++--
> tests/display-test.c | 2 +-
> tests/queue-test.c | 12 ++++++------
> tests/test-compositor.c | 4 ++--
> 6 files changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/src/scanner.c b/src/scanner.c
> index 04747e3..441ab1f 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -1267,10 +1267,10 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid
>
> if (side == CLIENT) {
> printf("static inline int\n"
> - "%s_add_listener(struct %s *%s,\n"
> + "%s_set_listener(struct %s *%s,\n"
> "%sconst struct %s_listener *listener, void *data)\n"
> "{\n"
> - "\treturn wl_proxy_add_listener((struct wl_proxy *) %s,\n"
> + "\treturn wl_proxy_set_listener((struct wl_proxy *) %s,\n"
> "%s(void (**)(void)) listener, data);\n"
> "}\n\n",
> interface->name, interface->name, interface->name,
> @@ -1278,6 +1278,22 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid
> interface->name,
> interface->name,
> indent(37));
> + printf("#ifndef WL_HIDE_DEPRECATED\n");
We cannot use WL_HIDE_DEPRECATED, because people may already be using
it, and then this change would break their code. I think we should look
into some other projects on how to deal with deprecation properly.
WL_HIDE_DEPRECATED worked only once, and no more functions can be moved
behind it.
> + printf("static inline int\n"
> + "%s_add_listener(struct %s *%s,\n"
> + "%sconst struct %s_listener *listener, void *data) WL_DEPRECATED;\n",
> + interface->name, interface->name, interface->name,
> + indent(14 + strlen(interface->name)), interface->name);
We can't use function attributes on definitions, only on declarations?
> + printf("static inline int\n"
> + "%s_add_listener(struct %s *%s,\n"
> + "%sconst struct %s_listener *listener, void *data)\n"
> + "{\n"
> + "\treturn %s_set_listener(%s, listener, data);\n"
> + "}\n",
> + interface->name, interface->name, interface->name,
> + indent(14 + strlen(interface->name)), interface->name,
> + interface->name, interface->name);
> + printf("#endif //WL_HIDE_DEPRECATED\n\n");
> }
> }
>
Otherwise the change to scanner looks fine.
We should probably think of a standard procedure in making deprecations
to the API. Who would like to take that task?
It would be three parts: compile time warnings, hiding API from
libwayland headers, and removing API from scanner-generated headers. I
think we might need some kind of a running version form of
WL_HIDE_DEPRECATED, one for each release, plus an option for scanner to
"stop emitting wrappers for things deprecated up to and including
release 1.x". Or should we just forget about hiding/removing deprecated
API?
Please start a new thread about this.
> diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
> index 91f7e7b..f748a42 100644
> --- a/src/wayland-client-core.h
> +++ b/src/wayland-client-core.h
> @@ -159,7 +159,7 @@ void
> wl_proxy_destroy(struct wl_proxy *proxy);
>
> int
> -wl_proxy_add_listener(struct wl_proxy *proxy,
> +wl_proxy_set_listener(struct wl_proxy *proxy,
> void (**implementation)(void), void *data);
>
> const void *
> @@ -251,6 +251,16 @@ wl_display_read_events(struct wl_display *display);
> void
> wl_log_set_handler_client(wl_log_func_t handler);
>
> +
> +/* Deprecated functions below */
> +#ifndef WL_HIDE_DEPRECATED
Again, cannot do this.
> +
> +int
> +wl_proxy_add_listener(struct wl_proxy *proxy,
> + void (**implementation)(void), void *data) WL_DEPRECATED;
> +
> +#endif
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 297c7a5..fe729b0 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -458,7 +458,7 @@ wl_proxy_destroy(struct wl_proxy *proxy)
> * \memberof wl_proxy
> */
> WL_EXPORT int
> -wl_proxy_add_listener(struct wl_proxy *proxy,
> +wl_proxy_set_listener(struct wl_proxy *proxy,
> void (**implementation)(void), void *data)
> {
> if (proxy->object.implementation || proxy->dispatcher) {
> @@ -1094,7 +1094,7 @@ wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *qu
> if (callback == NULL)
> return -1;
> wl_proxy_set_queue((struct wl_proxy *) callback, queue);
> - wl_callback_add_listener(callback, &sync_listener, &done);
> + wl_callback_set_listener(callback, &sync_listener, &done);
Here is the easy conflict with the proxy wrappers changes.
> while (!done && ret >= 0)
> ret = wl_display_dispatch_queue(display, queue);
>
> @@ -1960,3 +1960,22 @@ wl_log_set_handler_client(wl_log_func_t handler)
> {
> wl_log_handler = handler;
> }
> +
> +
> +/** \cond */ /* Deprecated functions below. */
> +
> +/** Deprecated. Use \a wl_proxy_set_listener instead.
> + */
> +WL_EXPORT int
> +wl_proxy_add_listener(struct wl_proxy *proxy,
> + void (**implementation)(void), void *data)
> +{
> + return wl_proxy_set_listener(proxy, implementation, data);
> +}
> +
> +/** \endcond */
> +
> +/* Functions at the end of this file are deprecated. Instead of adding new
> + * code here, add it before the comment above that states:
> + * Deprecated functions below.
> + */
Seems fine to me.
> diff --git a/tests/display-test.c b/tests/display-test.c
> index f9f8160..d7e1fd8 100644
> --- a/tests/display-test.c
> +++ b/tests/display-test.c
> @@ -161,7 +161,7 @@ client_get_seat_with_info(struct client *c, struct handler_info *hi)
>
> assert(hi);
> hi->seat = NULL;
> - wl_registry_add_listener(reg, ®istry_listener, hi);
> + wl_registry_set_listener(reg, ®istry_listener, hi);
> wl_display_roundtrip(c->wl_display);
> assert(hi->seat);
>
> diff --git a/tests/queue-test.c b/tests/queue-test.c
> index 02865ae..48927a4 100644
> --- a/tests/queue-test.c
> +++ b/tests/queue-test.c
> @@ -67,7 +67,7 @@ client_test_proxy_destroy(void)
>
> registry = wl_display_get_registry(display);
> assert(registry != NULL);
> - wl_registry_add_listener(registry, ®istry_listener,
> + wl_registry_set_listener(registry, ®istry_listener,
> &counter);
> assert(wl_display_roundtrip(display) != -1);
>
> @@ -121,12 +121,12 @@ client_test_multiple_queues(void)
> state.done = false;
> callback1 = wl_display_sync(state.display);
> assert(callback1 != NULL);
> - wl_callback_add_listener(callback1, &sync_listener, &state);
> + wl_callback_set_listener(callback1, &sync_listener, &state);
> wl_proxy_set_queue((struct wl_proxy *) callback1, queue);
>
> state.callback2 = wl_display_sync(state.display);
> assert(state.callback2 != NULL);
> - wl_callback_add_listener(state.callback2, &sync_listener, NULL);
> + wl_callback_set_listener(state.callback2, &sync_listener, NULL);
> wl_proxy_set_queue((struct wl_proxy *) state.callback2, queue);
>
> wl_display_flush(state.display);
> @@ -172,12 +172,12 @@ client_test_queue_roundtrip(void)
> /* arm a callback on the default queue */
> callback1 = wl_display_sync(display);
> assert(callback1 != NULL);
> - wl_callback_add_listener(callback1, &sync_listener_roundtrip, &done1);
> + wl_callback_set_listener(callback1, &sync_listener_roundtrip, &done1);
>
> /* arm a callback on the other queue */
> callback2 = wl_display_sync(display);
> assert(callback2 != NULL);
> - wl_callback_add_listener(callback2, &sync_listener_roundtrip, &done2);
> + wl_callback_set_listener(callback2, &sync_listener_roundtrip, &done2);
> wl_proxy_set_queue((struct wl_proxy *) callback2, queue);
>
> /* roundtrip on default queue must not dispatch the other queue. */
> @@ -191,7 +191,7 @@ client_test_queue_roundtrip(void)
> done1 = false;
> callback1 = wl_display_sync(display);
> assert(callback1 != NULL);
> - wl_callback_add_listener(callback1, &sync_listener_roundtrip, &done1);
> + wl_callback_set_listener(callback1, &sync_listener_roundtrip, &done1);
>
> wl_display_roundtrip_queue(display, queue);
> assert(done1 == false);
> diff --git a/tests/test-compositor.c b/tests/test-compositor.c
> index b01e8af..9170957 100644
> --- a/tests/test-compositor.c
> +++ b/tests/test-compositor.c
> @@ -429,7 +429,7 @@ registry_handle_globals(void *data, struct wl_registry *registry,
> c->tc = wl_registry_bind(registry, id, &test_compositor_interface, ver);
> assert(c->tc && "Failed binding to registry");
>
> - wl_proxy_add_listener((struct wl_proxy *) c->tc,
> + wl_proxy_set_listener((struct wl_proxy *) c->tc,
> (void *) &tc_listener, c);
> }
>
> @@ -452,7 +452,7 @@ struct client *client_connect()
> * registry so that client can define it's own listener later */
> reg = wl_display_get_registry(c->wl_display);
> assert(reg);
> - wl_registry_add_listener(reg, ®istry_listener, c);
> + wl_registry_set_listener(reg, ®istry_listener, c);
> wl_display_roundtrip(c->wl_display);
> assert(c->tc);
>
The rest is trivially correct.
So, maybe we could have this patch without the WL_HIDE_DEPRECATED bits
now, and then figure out how we are going to deprecate and break the
API (not ABI) in a tractable way?
(I think libwayland-client is a low-level enough library that we can
never break ABI, especially considering you cannot use different
versions of it in the same process and EGL implementation use it and so
forth, so even statically linking it won't help forgotten binary-only
programs.)
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160617/0904f7ab/attachment-0001.sig>
More information about the wayland-devel
mailing list