Hi, On 5 February 2015 at 08:58, Jonas Ådahl <[email protected]> wrote: > If a client calls xdg_shell.get_xdg_surface on a surface that is already > an xdg_surface wold, prior to this patch, succeed, but cause weston to > crash later when trying to configure. This patch instead sends a role > error to the client complaining that it already is an xdg_surface. > > Note that .._set_role() only fails when changing roles, not when setting > the same role twice.
Yeah, that one was interesting to me. I wonder if there's any benefit to making it fail unconditionally if we try to re-set the same role - but that's a whole different patchset. > The same is done for xdg_popup. > > Signed-off-by: Jonas Ådahl <[email protected]> > --- > desktop-shell/shell.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c > index a7514f7..cc345b5 100644 > --- a/desktop-shell/shell.c > +++ b/desktop-shell/shell.c > @@ -3959,6 +3959,14 @@ xdg_get_xdg_surface(struct wl_client *client, > struct desktop_shell *shell = sc->shell; > struct shell_surface *shsurf; > > + if ((shsurf = get_shell_surface(surface)) && Please assign shsurf in the declarations above - I hate assignments in if statements. > + shell_surface_is_xdg_surface(shsurf)) { > + wl_resource_post_error(resource, XDG_SHELL_ERROR_ROLE, > + "This wl_surface is already an " > + "xdg_surface"); > + return; > + } > + > if (weston_surface_set_role(surface, "xdg_surface", > resource, XDG_SHELL_ERROR_ROLE) < 0) > return; > @@ -4052,6 +4060,14 @@ xdg_get_xdg_popup(struct wl_client *client, > struct weston_surface *parent; > struct shell_seat *seat; > > + if ((shsurf = get_shell_surface(surface)) && Ditto. Ultimately I'd like to see weston_surface_set_role() deal with this and not have to have the split codepaths, but not for 1.7. With those two nitpicks fixed: Reviewed-by: Daniel Stone <[email protected]> Cheers, Daniel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
