From 89cb4842208a2997cfa3a9bef7ac66e01a414ebb Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Fri, 23 Jun 2023 14:23:27 +0200 Subject: [PATCH] compositor: replace role_data with role_resource This increases type safety, makes it more obvious that role_data must represent the role object, and will allow for automatic cleanup when the resource is destroyed. --- include/wlr/types/wlr_compositor.h | 4 ++-- types/wlr_compositor.c | 20 ++++++++++---------- types/wlr_input_method_v2.c | 24 +++++++++++++----------- types/wlr_layer_shell_v1.c | 6 +++--- types/wlr_session_lock_v1.c | 6 +++--- types/wlr_subcompositor.c | 6 +++--- types/xdg_shell/wlr_xdg_popup.c | 2 +- types/xdg_shell/wlr_xdg_surface.c | 5 ++++- types/xdg_shell/wlr_xdg_toplevel.c | 2 +- xwayland/shell.c | 6 +++--- 10 files changed, 43 insertions(+), 38 deletions(-) diff --git a/include/wlr/types/wlr_compositor.h b/include/wlr/types/wlr_compositor.h index ba2d4649..40ea7e9f 100644 --- a/include/wlr/types/wlr_compositor.h +++ b/include/wlr/types/wlr_compositor.h @@ -167,7 +167,7 @@ struct wlr_surface { * The role object representing the role. NULL if the role isn't * represented by any object or the object was destroyed. */ - void *role_data; + struct wl_resource *role_resource; struct { struct wl_signal client_commit; @@ -247,7 +247,7 @@ bool wlr_surface_set_role(struct wlr_surface *surface, const struct wlr_surface_ * Set the role object for this surface. The surface must have a role and * no already set role object. */ -void wlr_surface_set_role_object(struct wlr_surface *surface, void *role_data); +void wlr_surface_set_role_object(struct wlr_surface *surface, struct wl_resource *role_resource); /** * Destroy the object representing the surface's role. If it doesn't exist, diff --git a/types/wlr_compositor.c b/types/wlr_compositor.c index 1a135301..626fc9d8 100644 --- a/types/wlr_compositor.c +++ b/types/wlr_compositor.c @@ -37,7 +37,7 @@ static int max(int fst, int snd) { static void surface_handle_destroy(struct wl_client *client, struct wl_resource *resource) { struct wlr_surface *surface = wlr_surface_from_resource(resource); - if (surface->role_data != NULL) { + if (surface->role_resource != NULL) { wl_resource_post_error(resource, WL_SURFACE_ERROR_DEFUNCT_ROLE_OBJECT, "surface was destroyed before its role object"); @@ -492,7 +492,7 @@ static void surface_commit_state(struct wlr_surface *surface, } if (surface->role != NULL && surface->role->commit != NULL && - (surface->role_data != NULL || surface->role->no_object)) { + (surface->role_resource != NULL || surface->role->no_object)) { surface->role->commit(surface); } @@ -753,7 +753,7 @@ void wlr_surface_unmap(struct wlr_surface *surface) { surface->mapped = false; wl_signal_emit_mutable(&surface->events.unmap, NULL); if (surface->role != NULL && surface->role->unmap != NULL && - (surface->role_data != NULL || surface->role->no_object)) { + (surface->role_resource != NULL || surface->role->no_object)) { surface->role->unmap(surface); } @@ -779,7 +779,7 @@ bool wlr_surface_set_role(struct wlr_surface *surface, const struct wlr_surface_ } return false; } - if (surface->role_data != NULL) { + if (surface->role_resource != NULL) { wl_resource_post_error(error_resource, error_code, "Cannot reassign role %s to wl_surface@%" PRIu32 ", role object still exists", role->name, wl_resource_get_id(surface->resource)); @@ -790,23 +790,23 @@ bool wlr_surface_set_role(struct wlr_surface *surface, const struct wlr_surface_ return true; } -void wlr_surface_set_role_object(struct wlr_surface *surface, void *role_data) { +void wlr_surface_set_role_object(struct wlr_surface *surface, struct wl_resource *role_resource) { assert(surface->role != NULL); assert(!surface->role->no_object); - assert(surface->role_data == NULL); - assert(role_data != NULL); - surface->role_data = role_data; + assert(surface->role_resource == NULL); + assert(role_resource != NULL); + surface->role_resource = role_resource; } void wlr_surface_destroy_role_object(struct wlr_surface *surface) { - if (surface->role_data == NULL) { + if (surface->role_resource == NULL) { return; } wlr_surface_unmap(surface); if (surface->role->destroy != NULL) { surface->role->destroy(surface); } - surface->role_data = NULL; + surface->role_resource = NULL; } uint32_t wlr_surface_lock_pending(struct wlr_surface *surface) { diff --git a/types/wlr_input_method_v2.c b/types/wlr_input_method_v2.c index f8c9d442..3658104e 100644 --- a/types/wlr_input_method_v2.c +++ b/types/wlr_input_method_v2.c @@ -138,12 +138,14 @@ static void popup_surface_consider_map(struct wlr_input_popup_surface_v2 *popup_ } static void popup_surface_surface_role_commit(struct wlr_surface *surface) { - struct wlr_input_popup_surface_v2 *popup_surface = surface->role_data; + struct wlr_input_popup_surface_v2 *popup_surface = + wlr_input_popup_surface_v2_try_from_wlr_surface(surface); popup_surface_consider_map(popup_surface); } static void popup_surface_surface_role_destroy(struct wlr_surface *surface) { - struct wlr_input_popup_surface_v2 *popup_surface = surface->role_data; + struct wlr_input_popup_surface_v2 *popup_surface = + wlr_input_popup_surface_v2_try_from_wlr_surface(surface); wl_signal_emit_mutable(&popup_surface->events.destroy, NULL); wl_list_remove(&popup_surface->link); @@ -157,14 +159,6 @@ static const struct wlr_surface_role input_popup_surface_v2_role = { .destroy = popup_surface_surface_role_destroy, }; -struct wlr_input_popup_surface_v2 *wlr_input_popup_surface_v2_try_from_wlr_surface( - struct wlr_surface *surface) { - if (surface->role != &input_popup_surface_v2_role) { - return NULL; - } - return (struct wlr_input_popup_surface_v2 *)surface->role_data; -} - static const struct zwp_input_popup_surface_v2_interface input_popup_impl; static struct wlr_input_popup_surface_v2 *popup_surface_from_resource(struct wl_resource *resource) { @@ -173,6 +167,14 @@ static struct wlr_input_popup_surface_v2 *popup_surface_from_resource(struct wl_ return wl_resource_get_user_data(resource); } +struct wlr_input_popup_surface_v2 *wlr_input_popup_surface_v2_try_from_wlr_surface( + struct wlr_surface *surface) { + if (surface->role != &input_popup_surface_v2_role || surface->role_resource == NULL) { + return NULL; + } + return popup_surface_from_resource(surface->role_resource); +} + static void popup_resource_destroy(struct wl_resource *resource) { struct wlr_input_popup_surface_v2 *popup_surface = popup_surface_from_resource(resource); @@ -225,7 +227,7 @@ static void im_get_input_popup_surface(struct wl_client *client, wl_resource_set_implementation(popup_resource, &input_popup_impl, popup_surface, popup_resource_destroy); - wlr_surface_set_role_object(surface, popup_surface); + wlr_surface_set_role_object(surface, popup_resource); popup_surface->resource = popup_resource; popup_surface->input_method = input_method; diff --git a/types/wlr_layer_shell_v1.c b/types/wlr_layer_shell_v1.c index b4348f7e..5e6e83eb 100644 --- a/types/wlr_layer_shell_v1.c +++ b/types/wlr_layer_shell_v1.c @@ -40,10 +40,10 @@ static const struct wlr_surface_role layer_surface_role; struct wlr_layer_surface_v1 *wlr_layer_surface_v1_try_from_wlr_surface( struct wlr_surface *surface) { - if (surface->role != &layer_surface_role) { + if (surface->role != &layer_surface_role || surface->role_resource == NULL) { return NULL; } - return (struct wlr_layer_surface_v1 *)surface->role_data; + return wlr_layer_surface_v1_from_resource(surface->role_resource); } static void layer_surface_configure_destroy( @@ -445,7 +445,7 @@ static void layer_shell_handle_get_layer_surface(struct wl_client *wl_client, wl_resource_set_implementation(surface->resource, &layer_surface_implementation, surface, layer_surface_resource_destroy); - wlr_surface_set_role_object(wlr_surface, surface); + wlr_surface_set_role_object(wlr_surface, surface->resource); } static const struct zwlr_layer_shell_v1_interface layer_shell_implementation = { diff --git a/types/wlr_session_lock_v1.c b/types/wlr_session_lock_v1.c index 69205804..cf79cd24 100644 --- a/types/wlr_session_lock_v1.c +++ b/types/wlr_session_lock_v1.c @@ -50,10 +50,10 @@ static const struct wlr_surface_role lock_surface_role; struct wlr_session_lock_surface_v1 *wlr_session_lock_surface_v1_try_from_wlr_surface( struct wlr_surface *surface) { - if (surface->role != &lock_surface_role) { + if (surface->role != &lock_surface_role || surface->role_resource == NULL) { return NULL; } - return (struct wlr_session_lock_surface_v1 *)surface->role_data; + return lock_surface_from_resource(surface->role_resource); } uint32_t wlr_session_lock_surface_v1_configure( @@ -274,7 +274,7 @@ static void lock_handle_get_lock_surface(struct wl_client *client, lock_surface->resource = lock_surface_resource; wl_resource_set_user_data(lock_surface_resource, lock_surface); - wlr_surface_set_role_object(surface, lock_surface); + wlr_surface_set_role_object(surface, lock_surface_resource); wl_list_insert(&lock->surfaces, &lock_surface->link); diff --git a/types/wlr_subcompositor.c b/types/wlr_subcompositor.c index 51a7baa9..8270c86e 100644 --- a/types/wlr_subcompositor.c +++ b/types/wlr_subcompositor.c @@ -293,10 +293,10 @@ void subsurface_handle_parent_commit(struct wlr_subsurface *subsurface) { } struct wlr_subsurface *wlr_subsurface_try_from_wlr_surface(struct wlr_surface *surface) { - if (surface->role != &subsurface_role) { + if (surface->role != &subsurface_role || surface->role_resource == NULL) { return NULL; } - return (struct wlr_subsurface *)surface->role_data; + return subsurface_from_resource(surface->role_resource); } static void subcompositor_handle_destroy(struct wl_client *client, @@ -344,7 +344,7 @@ static void subcompositor_handle_get_subsurface(struct wl_client *client, wl_resource_set_implementation(subsurface->resource, &subsurface_implementation, subsurface, subsurface_resource_destroy); - wlr_surface_set_role_object(surface, subsurface); + wlr_surface_set_role_object(surface, subsurface->resource); wl_signal_init(&subsurface->events.destroy); diff --git a/types/xdg_shell/wlr_xdg_popup.c b/types/xdg_shell/wlr_xdg_popup.c index 4ef83ebf..d3600307 100644 --- a/types/xdg_shell/wlr_xdg_popup.c +++ b/types/xdg_shell/wlr_xdg_popup.c @@ -412,7 +412,7 @@ void create_xdg_popup(struct wlr_xdg_surface *surface, &xdg_popup_implementation, surface->popup, xdg_popup_handle_resource_destroy); - wlr_surface_set_role_object(surface->surface, surface); + wlr_surface_set_role_object(surface->surface, surface->resource); surface->role = WLR_XDG_SURFACE_ROLE_POPUP; diff --git a/types/xdg_shell/wlr_xdg_surface.c b/types/xdg_shell/wlr_xdg_surface.c index ebe8dcd5..d0ac4287 100644 --- a/types/xdg_shell/wlr_xdg_surface.c +++ b/types/xdg_shell/wlr_xdg_surface.c @@ -11,7 +11,10 @@ struct wlr_xdg_surface *wlr_xdg_surface_try_from_wlr_surface( surface->role != &xdg_popup_surface_role) { return NULL; } - return (struct wlr_xdg_surface *)surface->role_data; + if (surface->role_resource == NULL) { + return NULL; + } + return wlr_xdg_surface_from_resource(surface->role_resource); } static void xdg_surface_configure_destroy( diff --git a/types/xdg_shell/wlr_xdg_toplevel.c b/types/xdg_shell/wlr_xdg_toplevel.c index 73a0f496..cc3a31c3 100644 --- a/types/xdg_shell/wlr_xdg_toplevel.c +++ b/types/xdg_shell/wlr_xdg_toplevel.c @@ -524,7 +524,7 @@ void create_xdg_toplevel(struct wlr_xdg_surface *surface, &xdg_toplevel_implementation, surface->toplevel, xdg_toplevel_handle_resource_destroy); - wlr_surface_set_role_object(surface->surface, surface); + wlr_surface_set_role_object(surface->surface, surface->resource); surface->role = WLR_XDG_SURFACE_ROLE_TOPLEVEL; } diff --git a/xwayland/shell.c b/xwayland/shell.c index d362cb7d..28f572d3 100644 --- a/xwayland/shell.c +++ b/xwayland/shell.c @@ -38,7 +38,7 @@ static struct wlr_xwayland_surface_v1 *xwl_surface_from_resource( } static void xwl_surface_role_commit(struct wlr_surface *surface) { - struct wlr_xwayland_surface_v1 *xwl_surface = surface->role_data; + struct wlr_xwayland_surface_v1 *xwl_surface = xwl_surface_from_resource(surface->role_resource); if (xwl_surface->serial != 0 && !xwl_surface->added) { xwl_surface->added = true; @@ -48,7 +48,7 @@ static void xwl_surface_role_commit(struct wlr_surface *surface) { } static void xwl_surface_role_destroy(struct wlr_surface *surface) { - struct wlr_xwayland_surface_v1 *xwl_surface = surface->role_data; + struct wlr_xwayland_surface_v1 *xwl_surface = xwl_surface_from_resource(surface->role_resource); wl_list_remove(&xwl_surface->surface_destroy.link); wl_list_remove(&xwl_surface->link); wl_resource_set_user_data(xwl_surface->resource, NULL); // make inert @@ -131,7 +131,7 @@ static void shell_handle_get_xwayland_surface(struct wl_client *client, wl_resource_set_implementation(xwl_surface->resource, &xwl_surface_impl, xwl_surface, xwl_surface_handle_resource_destroy); - wlr_surface_set_role_object(surface, xwl_surface); + wlr_surface_set_role_object(surface, xwl_surface->resource); wl_list_insert(&shell->surfaces, &xwl_surface->link);