[PATCH] rdp: don't release the seat until it is safe v2

[Sam Spilsbury]

On Fri, May 20, 2016 at 5:33 PM, David Fort <rdp.effort at gmail.com> wrote:
> Releasing a seat is not safe, so let's just announce it without keyboard
> and mouse until this is fixed. Without this patch we just can't reconnect on
> the RDP compositor as it crashes.
>
> v2: fixed the leak of the xkb_keymap
>
> Signed-off-by: David Fort <contact at hardening-consulting.com>
> ---
>  src/compositor-rdp.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 4fc7c74..52cf426 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -109,7 +109,7 @@ enum peer_item_flags {
>  struct rdp_peers_item {
>         int flags;
>         freerdp_peer *peer;
> -       struct weston_seat seat;
> +       struct weston_seat *seat;
>
>         struct wl_list link;
>  };
> @@ -640,9 +640,9 @@ rdp_peer_context_free(freerdp_peer* client, RdpPeerContext* context)
>         }
>
>         if (context->item.flags & RDP_PEER_ACTIVATED) {
> -               weston_seat_release_keyboard(&context->item.seat);
> -               weston_seat_release_pointer(&context->item.seat);
> -               weston_seat_release(&context->item.seat);
> +               weston_seat_release_keyboard(context->item.seat);
> +               weston_seat_release_pointer(context->item.seat);
> +               /*weston_seat_release(context->item.seat);*/
>         }

I think instead of just having commented out code, put the reasons why
the seat cannot be released safely at the moment. Just having it
commented out will confuse future maintainers.

>
>         Stream_Free(context->encode_stream, TRUE);
> @@ -911,9 +911,16 @@ xf_peer_activate(freerdp_peer* client)
>         else
>                 snprintf(seat_name, sizeof(seat_name), "RDP peer @%s", settings->ClientAddress);
>
> -       weston_seat_init(&peersItem->seat, b->compositor, seat_name);
> -       weston_seat_init_keyboard(&peersItem->seat, keymap);
> -       weston_seat_init_pointer(&peersItem->seat);
> +       peersItem->seat = zalloc(sizeof(*peersItem->seat));
> +       if (!peersItem->seat) {
> +               xkb_keymap_unref(keymap);
> +               weston_log("unable to create a weston_seat\n");
> +               return FALSE;
> +       }
> +
> +       weston_seat_init(peersItem->seat, b->compositor, seat_name);
> +       weston_seat_init_keyboard(peersItem->seat, keymap);
> +       weston_seat_init_pointer(peersItem->seat);

Any reason to make this dynamically allocated memory? It seems to me
like it is adding an additional point of failure for little added
benefit. If it needs to be dynamically allocated, I think such a
change should come in a separate patch, since it seems unrelated to
this one.

Reviewed-by: Sam Spilsbury <smspillaz at gmail.com>