[PATCH weston v2] desktop-shell: Fail if get_xdg_surface is called on an xdg_surface

[Bill Spitzak]

How about a wrapper function around weston_surface_set_role that always 
fails if there is a non-zero role, and then call that? You can then 
gradually transition all the other calls to this new one, and if they 
are all changed then remove the old function.

On 02/05/2015 05:21 AM, Jonas Ådahl 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.
>
> The same is done for xdg_popup.
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> ---
>
> Changed since v1:
>
> Put the assignments on their own lines.
>
>
>   desktop-shell/shell.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index a7514f7..d4407d0 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;
>
> +	shsurf = get_shell_surface(surface);
> +	if (shsurf && 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;
>
> +	shsurf = get_shell_surface(surface);
> +	if (shsurf && shell_surface_is_xdg_popup(shsurf)) {
> +		wl_resource_post_error(resource, XDG_SHELL_ERROR_ROLE,
> +				       "This wl_surface is already an "
> +				       "xdg_popup");
> +		return;
> +	}
> +
>   	if (weston_surface_set_role(surface, "xdg_popup",
>   				    resource, XDG_SHELL_ERROR_ROLE) < 0)
>   		return;
>