On Wed, 10 Feb 2016 17:35:40 +0200 Pekka Paalanen <[email protected]> wrote:
> Hi, > > the subject should be: > [PATCH wayland] server: add listener API for new clients > > On Tue, 9 Feb 2016 17:31:27 +0200 > Giulio Camuffo <[email protected]> wrote: > > > Hi, > > > > thanks, i think this was long overdue. I have a few comments below. > > Indeed. > > > 2016-02-04 11:59 GMT+02:00 <[email protected]>: > > > Using display object, if a new client is created, emit a signal. > > > > > > In the server-side, we can get the destroy event of a client, > > > > The indentation is weird from here on. > > Yeah, this looks like its been copied from git-log or git-show output. > > > > But there is no way to get the client create event. > > > Of course, we can get the client object from the global registry > > > binding callbacks. > > > But it can be called several times with same client object. > > > And even if A client creates display object, > > > (so there is a connection), The server could not know that. > > > There could be more use-cases not only for this. > > > > > > Please review this and if possible, merge it to the stream. > > > > I think this line is for the list, it should not be in the commit message. > > > > > > > > Signed-off-by: Sung-jae Park <[email protected]> > > > > > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h > > > index e8e1e9c..cb72981 100644 > > > --- a/src/wayland-server-core.h > > > +++ b/src/wayland-server-core.h > > > @@ -156,6 +156,21 @@ void > > > wl_display_add_destroy_listener(struct wl_display *display, > > > struct wl_listener *listener); > > > > > > +/** Add a listener for getting a notification of creation of clients. > > > + * If you add a listener, server will emits a signal if a new client > > > + * is created. > > > + * > > > + * \ref wl_client_create > > > + * \ref wl_display > > > + * \ref wl_listener > > > + * > > > + * \param display The display object > > > + * \param listener Signal handler object > > Please document what argument will be passed to the notification > function when the signal is emitted. > > > > + */ > > > > The documentation for the function should be in the .c file. > > > > > +void > > > +wl_display_add_create_client_listener(struct wl_display *display, > > > + struct wl_listener *listener); > > > > I think i would prefer wl_display_add_client_connected_listener, it > > flows better and makes it more clear that this is about when a client > > connects to the display. Also i would reword the docs a bit, how about > > " Registers a listener for the client connection signal. > > When a client connects to the display the \a listener will be > > notified, carrying > > a pointer to the new wl_client object." > > > > Agreed. > > Another name could be wl_display_add_client_created_listener(). We have > two cases: one is when clients connect through a socket, and the other > is when clients get created by calling wl_client_create() directly with > an fd from socketpair(). "created" would match wl_client_create() > better. > > > > + > > > struct wl_listener * > > > wl_display_get_destroy_listener(struct wl_display *display, > > > wl_notify_func_t notify); > > > diff --git a/src/wayland-server.c b/src/wayland-server.c > > > index ae9365f..0eff8f6 100644 > > > --- a/src/wayland-server.c > > > +++ b/src/wayland-server.c > > > @@ -96,6 +96,7 @@ struct wl_display { > > > struct wl_list client_list; > > > > > > struct wl_signal destroy_signal; > > > + struct wl_signal create_client_signal; > > > > > > struct wl_array additional_shm_formats; > > > }; > > > @@ -448,6 +449,8 @@ wl_client_create(struct wl_display *display, int fd) > > > > > > wl_list_insert(display->client_list.prev, &client->link); > > > > > > + wl_signal_emit(&display->create_client_signal, client); > > > + > > > return client; > > > > > > err_map: > > > @@ -864,6 +867,7 @@ wl_display_create(void) > > > wl_list_init(&display->registry_resource_list); > > > > > > wl_signal_init(&display->destroy_signal); > > > + wl_signal_init(&display->create_client_signal); > > > > > > display->id = 1; > > > display->serial = 0; > > > @@ -1353,6 +1357,13 @@ wl_display_add_destroy_listener(struct wl_display > > > *display, > > > wl_signal_add(&display->destroy_signal, listener); > > > } > > > > > > +WL_EXPORT void > > > +wl_display_add_create_client_listener(struct wl_display *display, > > > + struct wl_listener *listener) > > > +{ > > > + wl_signal_add(&display->create_client_signal, listener); > > > +} > > > + > > > WL_EXPORT struct wl_listener * > > > wl_display_get_destroy_listener(struct wl_display *display, > > > wl_notify_func_t notify) > > The idea seems good to me. Hi, one more thing I forgot. Could you write at least one test for this new feature, please? I think you could use TEST(post_error_to_one_client) in tests/display-test.c as an example. You don't have to create any globals, and the client function only needs to connect and disconnect. That should allow you to excercise the new listener. That would be follow-up patch. Thanks, pq
pgpd5ADqxdokL.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
