[PATCH wayland] server: Don't expose wl_display as a global

[Pekka Paalanen]

On Fri, 8 Aug 2014 14:55:43 -0700
Jason Ekstrand <jason at jlekstrand.net> wrote:

> I think this is ok.  To my knowledge, no one is re-binding wl_display or
> even relying on that global being exposed.

I cannot imagine any reason to advertize wl_display as a global
either. Personally I have always thought the version of wl_display
is immutable, and if the re-binding really is as broken as you say,
good riddance.

But, I would still like to have Kristian's comment here, so we're
not forgetting some detail on why it was there in the first place.


Thanks,
pq


> On Thu, Aug 7, 2014 at 6:55 AM, Jasper St. Pierre <jstpierre at 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;
> >  }
> >
> > --
> > 2.0.4