[PATCH] shell: Check more thoroughly for undefined surface resource

[Daniel Stone]

Hi,

On 14 September 2015 at 20:31, Bryce Harrington <bryce at osg.samsung.com> wrote:
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index ecc42c5..0b91ce6 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2148,7 +2148,7 @@ xdg_ping_timeout_handler(void *data)
>                         continue;
>
>                 shsurf = get_shell_surface(pointer->focus->surface);
> -               if (shsurf &&
> +               if (shsurf && grab->shsurf->resource &&

This should be shsurf->resource.

>                     wl_resource_get_client(shsurf->resource) == sc->client)
>                         set_busy_cursor(shsurf, pointer);
>         }
> @@ -2183,7 +2183,8 @@ handle_xdg_ping(struct shell_surface *shsurf, uint32_t serial)
>         if (shell_surface_is_xdg_surface(shsurf) ||
>             shell_surface_is_xdg_popup(shsurf))
>                 xdg_shell_send_ping(sc->resource, serial);
> -       else if (shell_surface_is_wl_shell_surface(shsurf))
> +       else if (shell_surface_is_wl_shell_surface(shsurf)
> +                && grab->shsurf->resource)
>                 wl_shell_surface_send_ping(shsurf->resource, serial);
>  }

Ditto.

> @@ -3372,7 +3373,9 @@ static const struct weston_touch_grab_interface touch_popup_grab_interface = {
>  static void
>  shell_surface_send_popup_done(struct shell_surface *shsurf)
>  {
> -       if (shell_surface_is_wl_shell_surface(shsurf))
> +       if (grab->shsurf->resource == NULL)
> +               return;
> +       else if (shell_surface_is_wl_shell_surface(shsurf))
>                 wl_shell_surface_send_popup_done(shsurf->resource);
>         else if (shell_surface_is_xdg_popup(shsurf))
>                 xdg_popup_send_popup_done(shsurf->resource);

Ditto.

> @@ -3455,6 +3458,12 @@ add_popup_grab(struct shell_surface *shsurf,
>         struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>         struct weston_touch *touch = weston_seat_get_touch(seat);
>
> +       if (shsurf->resource == NULL) {
> +               wl_resource_post_error(shsurf->owner_resource,
> +                                      WL_DISPLAY_ERROR_INVALID_OBJECT,
> +                                      "undefined resource when grabbing");
> +               return -1;
> +       }

This seems like most uses would be caught by the above checks. Is this
something that can/does happen? Does XWayland trigger it?

The rest seem OK to me:
Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel