On Thu, Aug 21, 2014 at 4:00 AM, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Thu, 7 Aug 2014 09:55:49 -0400 > "Jasper St. Pierre" <jstpie...@mecheye.net> wrote: > > > The idea here was that once upon a time, clients could rebind wl_display > > to a higher version, so we offered the ability to rebind it > > here. However, this is particularly broken. The existing bind > > implementation actually still hardcodes version numbers, and it leaks > > previous resources, overwriting the existing one. > > > > The newly bound resource *also* won't have any listeners attached by the > > client, meaning that the error and delete_id events won't get delivered > > correctly. Unless the client poked into libwayland internals, it also > > can't possibly set up these handlers correctly either, so the client > > will sustain errors and leak all deleted globals. > > > > Since this never worked correctly in the first place, we can feel safe > > removing it. > > --- > > src/wayland-server.c | 28 ++++++++-------------------- > > 1 file changed, 8 insertions(+), 20 deletions(-) > > > > diff --git a/src/wayland-server.c b/src/wayland-server.c > > index 3c162d4..dc3f502 100644 > > --- a/src/wayland-server.c > > +++ b/src/wayland-server.c > > @@ -380,9 +380,8 @@ wl_client_get_display(struct wl_client *client) > > return client->display; > > } > > > > -static void > > -bind_display(struct wl_client *client, > > - void *data, uint32_t version, uint32_t id); > > +static int > > +bind_display(struct wl_client *client, struct wl_display *display); > > > > /** Create a client for the given file descriptor > > * > > @@ -440,9 +439,7 @@ wl_client_create(struct wl_display *display, int fd) > > goto err_map; > > > > wl_signal_init(&client->destroy_signal); > > - bind_display(client, display, 1, 1); > > - > > - if (!client->display_resource) > > + if (bind_display(client, display) < 0) > > goto err_map; > > > > wl_list_insert(display->client_list.prev, &client->link); > > @@ -772,22 +769,20 @@ destroy_client_display_resource(struct wl_resource > *resource) > > resource->client->display_resource = NULL; > > } > > > > -static void > > -bind_display(struct wl_client *client, > > - void *data, uint32_t version, uint32_t id) > > +static int > > +bind_display(struct wl_client *client, struct wl_display *display) > > { > > - struct wl_display *display = data; > > - > > client->display_resource = > > - wl_resource_create(client, &wl_display_interface, 1, id); > > + wl_resource_create(client, &wl_display_interface, 1, 1); > > if (client->display_resource == NULL) { > > wl_client_post_no_memory(client); > > - return; > > + return -1; > > } > > > > wl_resource_set_implementation(client->display_resource, > > &display_interface, display, > > destroy_client_display_resource); > > + return 0; > > } > > > > /** Create Wayland display object. > > @@ -831,13 +826,6 @@ wl_display_create(void) > > > > wl_array_init(&display->additional_shm_formats); > > > > - if (!wl_global_create(display, &wl_display_interface, 1, > > - display, bind_display)) { > > - wl_event_loop_destroy(display->loop); > > - free(display); > > - return NULL; > > - } > > - > > return display; > > } > > > > I asked krh about this in irc, and he eventually agreed that exposing > wl_display as a global is not so useful. > > I reviewed the patch, and the alleged problems the current code has, > and I agree they are real. > > If we ever end up needing wl_display as a global, we can easily add it > back. Since the implementation was indeed broken, no client should ever > try to bind to wl_display global with version 1. > > Therefore, pushed with ack from Jason and R-b from me. > > In fact, I think this warrants this patch as a condidate for the stable > branch. > Yeah, it probably counts as a bug-fix. It does change behaviour though, so I'm not sure I really want to call it a stable thing. Unless, of course, re-binding wl_display could cause a crash; in which case, push it to 1.5 --Jason > > > Thanks, > pq >
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel