[PATCH weston] xwayland: Fix crash when run with no input device

[Giulio Camuffo]

Hi,

2016-11-01 2:38 GMT+01:00 Tom Hochstein <tom.hochstein at nxp.com>:
> Starting an xterm with no input device led to a crash
> because weston_wm_pick_seat() was returning garbage and
> weston_wm_selection_init() was trying to use the garbage.
>
> Signed-off-by: Tom Hochstein <tom.hochstein at nxp.com>
> ---
>  xwayland/selection.c      | 10 +++++++---
>  xwayland/window-manager.c |  6 ++++--
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/xwayland/selection.c b/xwayland/selection.c
> index 641ac49..60f9f20 100644
> --- a/xwayland/selection.c
> +++ b/xwayland/selection.c
> @@ -655,8 +655,10 @@ weston_wm_set_selection(struct wl_listener *listener, void *data)
>         struct weston_seat *seat = data;
>         struct weston_wm *wm =
>                 container_of(listener, struct weston_wm, selection_listener);
> -       struct weston_data_source *source = seat->selection_data_source;
> +       struct weston_data_source *source = NULL;
>
> +       if (seat != NULL)

There is no need to check seat here, see below.

> +               source = seat->selection_data_source;
>         if (source == NULL) {
>                 if (wm->selection_owner == wm->selection_window)
>                         xcb_set_selection_owner(wm->conn,
> @@ -709,8 +711,10 @@ weston_wm_selection_init(struct weston_wm *wm)
>                                           wm->atom.clipboard, mask);
>
>         seat = weston_wm_pick_seat(wm);
> -       wm->selection_listener.notify = weston_wm_set_selection;
> -       wl_signal_add(&seat->selection_signal, &wm->selection_listener);
> +       if (seat != NULL) {
> +               wm->selection_listener.notify = weston_wm_set_selection;
> +               wl_signal_add(&seat->selection_signal, &wm->selection_listener);
> +       }
>
>         weston_wm_set_selection(&wm->selection_listener, seat);

It doesn't make sense to call this with a NULL seat. You should
instead return early above if seat is NULL, and that will make sure
weston_wm_set_selection() will never be called with a NULL seat, so
you don't need to check inside there.
With that fixed this patch looks good to me, although it leaves the
hole open about what happens when a seat gets added later on: nothing.
The code should probably listen for the compositor's
seat_created_signal signal, and init the selection state for it.
However i think it can be a follow up, and this patch can just be a
fix for the crash.

Thanks,
Giulio

>  }
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 0e26d7c..15e2ca2 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -1306,8 +1306,10 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm, xcb_generic_event_t *even
>  struct weston_seat *
>  weston_wm_pick_seat(struct weston_wm *wm)
>  {
> -       return container_of(wm->server->compositor->seat_list.next,
> -                           struct weston_seat, link);
> +       struct wl_list *seats = wm->server->compositor->seat_list.next;
> +       if (wl_list_empty(seats))
> +               return NULL;
> +       return container_of(seats, struct weston_seat, link);
>  }
>
>  static struct weston_seat *
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel