On Fri, 26 Jan 2018 18:47:55 +0200 Alexandros Frantzis <[email protected]> wrote:
> Properly clean up all sub-objects (e.g., weston_pointer_client objects) > when a weston_pointer object is destroyed. The clean-up ensures that the > server is able to safely handle client requests to any associated > pointer resources, which, as a consenquence of a weston_pointer > destruction, have now become inert. > > The clean-up involves, among other things, unsetting the destroyed > weston_pointer object from the user data of pointer resources, and > handling this NULL user data case where required. > > Signed-off-by: Alexandros Frantzis <[email protected]> > --- > libweston/input.c | 41 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 5 deletions(-) Hi Alexandros, thank you for tackling these very old oversights. > diff --git a/libweston/input.c b/libweston/input.c > index 94a3423a..01f0d568 100644 > --- a/libweston/input.c > +++ b/libweston/input.c > @@ -105,6 +105,19 @@ weston_pointer_client_create(struct wl_client *client) > static void > weston_pointer_client_destroy(struct weston_pointer_client *pointer_client) > { > + struct wl_resource *resource; > + > + wl_resource_for_each(resource, &pointer_client->pointer_resources) { > + wl_resource_set_user_data(resource, NULL); > + } > + > + wl_resource_for_each(resource, > + &pointer_client->relative_pointer_resources) { > + wl_resource_set_user_data(resource, NULL); > + } > + > + wl_list_remove(&pointer_client->pointer_resources); > + wl_list_remove(&pointer_client->relative_pointer_resources); > free(pointer_client); > } > > @@ -170,11 +183,14 @@ unbind_pointer_client_resource(struct wl_resource > *resource) > struct wl_client *client = wl_resource_get_client(resource); > struct weston_pointer_client *pointer_client; > > - pointer_client = weston_pointer_get_pointer_client(pointer, client); > - assert(pointer_client); > - > wl_list_remove(wl_resource_get_link(resource)); > - weston_pointer_cleanup_pointer_client(pointer, pointer_client); > + > + if (pointer) { > + pointer_client = weston_pointer_get_pointer_client(pointer, > + client); > + assert(pointer_client); > + weston_pointer_cleanup_pointer_client(pointer, pointer_client); > + } > } > > static void unbind_resource(struct wl_resource *resource) > @@ -1092,12 +1108,18 @@ weston_pointer_create(struct weston_seat *seat) > WL_EXPORT void > weston_pointer_destroy(struct weston_pointer *pointer) > { > + struct weston_pointer_client *pointer_client, *tmp; > + > wl_signal_emit(&pointer->destroy_signal, pointer); > > if (pointer->sprite) > pointer_unmap_sprite(pointer); > > - /* XXX: What about pointer->resource_list? */ > + wl_list_for_each_safe(pointer_client, tmp, &pointer->pointer_clients, > + link) { > + wl_list_remove(&pointer_client->link); > + weston_pointer_client_destroy(pointer_client); > + } > > wl_list_remove(&pointer->focus_resource_listener.link); > wl_list_remove(&pointer->focus_view_listener.link); > @@ -2297,6 +2319,9 @@ pointer_set_cursor(struct wl_client *client, struct > wl_resource *resource, > struct weston_pointer *pointer = wl_resource_get_user_data(resource); > struct weston_surface *surface = NULL; > > + if (!pointer) > + return; > + Ideally if a wl_surface is provided, we would still do the role check and assignment, but I see that would require quite some rearranging here, so this is ok. > if (surface_resource) > surface = wl_resource_get_user_data(surface_resource); > > @@ -3674,6 +3699,9 @@ pointer_constraints_lock_pointer(struct wl_client > *client, > struct weston_region *region = region_resource ? > wl_resource_get_user_data(region_resource) : NULL; > > + if (!pointer) > + return; This doesn't work, because the request is creating a new protocol object. If you omit creating the zwp_locked_pointer_v1 protocol object (wl_resource), it will lead to the client and server disagreeing on what object ids are used and eventually to a fatal protocol error. > + > init_pointer_constraint(resource, id, surface, pointer, region, > lifetime, > &zwp_locked_pointer_v1_interface, > &locked_pointer_interface, > @@ -4467,6 +4495,9 @@ pointer_constraints_confine_pointer(struct wl_client > *client, > struct weston_region *region = region_resource ? > wl_resource_get_user_data(region_resource) : NULL; > > + if (!pointer) > + return; This one has the same problem as above. Maybe we should have a patch before this one to make init_pointer_constraint() deal with pointer=NULL? > + > init_pointer_constraint(resource, id, surface, pointer, region, > lifetime, > &zwp_confined_pointer_v1_interface, > &confined_pointer_interface, Otherwise this patch looks exactly correct. Thanks, pq
pgpefafdYlpso.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
