<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Aug 21, 2014 at 4:00 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, 7 Aug 2014 09:55:49 -0400 <div class="">"Jasper St. Pierre" <<a href="mailto:jstpierre@mecheye.net">jstpierre@mecheye.net</a>> wrote: </div><div><div class="h5">> 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; > } > </div></div>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. </blockquote><div> </div><div>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 </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks, pq </blockquote></div> </div></div>