[PATCH] RFC: server: implement wl_priv_signal_emit without emit_list

[Simon Ser]

Bump! Can you give me feedback please? Having a safe wl_signal_emit would greatly benefit libwayland users.
​---
Simon Ser
https://emersion.fr

-------- Original Message --------
On January 28, 2018 6:37 PM, Simon Ser <contact at emersion.fr> wrote:
>
>This is a RFC to be able to run tests and check that this approach
> is working. The end goal is to remove wl_priv_signal completely and
> implement a safe wl_signal_emit.
>
>What do you think of this approach? It passes all the tests.
> src/wayland-private.h |  1 -
> src/wayland-server.c  | 31 ++++++++++++++++---------------
> 2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index 12b9032..ad58226 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -238,7 +238,6 @@ zalloc(size_t s)
>
> struct wl_priv_signal {
> struct wl_list listener_list;
> - struct wl_list emit_list;
> };
>
> void
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 7225b4e..d587816 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1912,7 +1912,6 @@ void
> wl_priv_signal_init(struct wl_priv_signal *signal)
> {
> wl_list_init(&signal->listener_list);
>
> - wl_list_init(&signal->emit_list);
>}
>
> /** Add a listener to a signal
> @@ -1947,9 +1946,6 @@ wl_priv_signal_get(struct wl_priv_signal *signal, wl_notify_func_t notify)
> wl_list_for_each(l, &signal->listener_list, link)
>	if (l->notify == notify)
>
>		return l;
>
> - wl_list_for_each(l, &signal->emit_list, link)
>
> -   if (l->notify == notify)
>
>
> -   	return l;
>
>
> return NULL;
> }
> @@ -1966,25 +1962,30 @@ wl_priv_signal_emit(struct wl_priv_signal *signal, void *data)
> {
> struct wl_listener *l;
> struct wl_list *pos;
> - struct wl_listener cursor;
>
> - struct wl_listener end;
>
>
> - wl_list_insert_list(&signal->emit_list, &signal->listener_list);
>
> - wl_list_init(&signal->listener_list);
>
> - 
>
> - /* Take every element out of the list and put them in a temporary list.
>
> - 
> - This way, the 'it' func can remove any element it wants from the list
>
>
> - 
> - without troubles, because we always get the first element, not the
>
>
> - 
> - one after the current, which may be invalid.
>
>
> - /* Add two special markers: one cursor and one end marker. This way, we know
>
> - 
> - that we've already called listeners on the left of the cursor and that we
>
>
> - 
> - don't want to call listeners on the right of the end marker. The 'it'
>
>
> - 
> - function can remove any element it wants from the list without troubles.
>
> - wl_list_for_each_safe tries to be safe but it fails: it works fine
>
> - if the current item is removed, but not if the next one is. */
>
>
> - while (!wl_list_empty(&signal->emit_list)) {
>
> -   pos = signal->emit_list.next;
>
>
> - wl_list_insert(&signal->listener_list, &cursor.link);
>
> - wl_list_insert(signal->listener_list.prev, &end.link);
>
> - 
>
> - while (cursor.link.next != &end.link) {
>
> -   pos = cursor.link.next;
>  l = wl_container_of(pos, l, link);
>
>
> -   wl_list_remove(pos);
>
>
> -   wl_list_insert(&signal->listener_list, pos);
>
>
> -   wl_list_remove(&cursor.link);
>
>
> -   wl_list_insert(pos, &cursor.link);
>
>
> l->notify(l, data);
>}
>
> - 
>
> - wl_list_remove(&cursor.link);
>
> - wl_list_remove(&end.link);
> }
>
> /** \endcond INTERNAL */
> --
> 2.16.1
>
>
>