[PATCH weston v3] compositor: reset surface's resource field on resource destruction

[Kristian Høgsberg]

On Fri, Sep 13, 2013 at 11:54:59AM -0500, Jason Ekstrand wrote:
> Perhaps a better long-term solution would be to have the pointer register a
> destroy listener on the surface's resource and use that to null out the
> focus.  Then we wouldn't have to worry about specifically flagging this
> strange intermediate state.

We can even just set the surface->resource pointer to NULL before
calling weston_surface_destroy now.  With the check in
weston_keyboard/pointer_set_focus() we don't get the warning anymore.

Kristian

> --Jason Ekstrand
> On Sep 13, 2013 9:36 AM, "Giulio Camuffo" <giuliocamuffo at gmail.com> wrote:
> 
> > with the surface ref-count feature a surface may live on after its
> > resource was destroyed. set it to NULL in that case, so that code
> > like find_resource_for_surface() in input.c will act accordingly.
> > Moreover, we don't send the wl_pointer/keyboard_leave events if the
> > surface has a NULL resource.
> > ---
> >
> > Also fixed the silly write after free.
> >
> >  src/compositor.c |  4 ++++
> >  src/input.c      | 16 ++++++++++------
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 67ac64e..a1c01fb 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -1069,8 +1069,12 @@ static void
> >  destroy_surface(struct wl_resource *resource)
> >  {
> >         struct weston_surface *surface =
> > wl_resource_get_user_data(resource);
> > +       int destroyed = surface->ref_count <= 1;
> >
> >         weston_surface_destroy(surface);
> > +       if (!destroyed) {
> > +               surface->resource = NULL;
> > +       }
> >  }
> >
> >  static void
> > diff --git a/src/input.c b/src/input.c
> > index 9c30460..d6a4740 100644
> > --- a/src/input.c
> > +++ b/src/input.c
> > @@ -425,9 +425,11 @@ weston_pointer_set_focus(struct weston_pointer
> > *pointer,
> >
> >         resource = pointer->focus_resource;
> >         if (resource && pointer->focus != surface) {
> > -               serial = wl_display_next_serial(display);
> > -               wl_pointer_send_leave(resource, serial,
> > -                                     pointer->focus->resource);
> > +               if (pointer->focus->resource) {
> > +                       serial = wl_display_next_serial(display);
> > +                       wl_pointer_send_leave(resource, serial,
> > +                                               pointer->focus->resource);
> > +               }
> >                 wl_list_remove(&pointer->focus_listener.link);
> >         }
> >
> > @@ -471,9 +473,11 @@ weston_keyboard_set_focus(struct weston_keyboard
> > *keyboard,
> >
> >         if (keyboard->focus_resource && keyboard->focus != surface) {
> >                 resource = keyboard->focus_resource;
> > -               serial = wl_display_next_serial(display);
> > -               wl_keyboard_send_leave(resource, serial,
> > -                                      keyboard->focus->resource);
> > +               if (keyboard->focus->resource) {
> > +                       serial = wl_display_next_serial(display);
> > +                       wl_keyboard_send_leave(resource, serial,
> > +                                               keyboard->focus->resource);
> > +               }
> >                 wl_list_remove(&keyboard->focus_listener.link);
> >         }
> >
> > --
> > 1.8.4
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >

> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel