Re: [PATCH weston v2 3/7] libweston: Make weston_seat release safe

2018-02-13 Thread Quentin Glidic 

On 2/13/18 11:58 AM, Pekka Paalanen wrote:

On Thu,  8 Feb 2018 15:37:54 +0200
Alexandros Frantzis  wrote:


Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.

The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.

The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:

Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly

== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize

Signed-off-by: Alexandros Frantzis 
---
Changes in v2:
  - Properly create inert resources in seat_get_pointer/touch/keyboard.
  - Ensure all sites which have a wl_seat input resource can deal
with inert resources.

  compositor/text-backend.c|  8 --
  libweston-desktop/wl-shell.c | 12 +++-
  libweston-desktop/xdg-shell-v5.c | 16 ++-
  libweston-desktop/xdg-shell-v6.c | 18 +++-
  libweston/data-device.c  | 15 ++
  libweston/input.c| 61 
  6 files changed, 102 insertions(+), 28 deletions(-)


Hi Alf,

awesome work!

The changes look good to me, however I did not review the
libweston-desktop bits yet. I'm hoping Quentin would take a look at
those.

Therefore, apart from libweston-desktop bits, this patch is:
Reviewed-by: Pekka Paalanen 


Thanks,
pq


diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
index 66553f45..3386d48b 100644
--- a/libweston-desktop/wl-shell.c
+++ b/libweston-desktop/wl-shell.c
@@ -220,6 +220,9 @@ weston_desktop_wl_shell_surface_protocol_move(struct 
wl_client *wl_client,
struct weston_desktop_wl_shell_surface *surface =
weston_desktop_surface_get_implementation_data(dsurface);
  
+	if (seat == NULL)

+   return;
+
weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
  }
  
@@ -238,6 +241,9 @@ weston_desktop_wl_shell_surface_protocol_resize(struct wl_client *wl_client,

enum weston_desktop_surface_edge surf_edges =
(enum weston_desktop_surface_edge) edges;
  
+	if (seat == NULL)

+   return;
+
weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, 
surf_edges);
  }
  
@@ -321,13 +327,17 @@ weston_desktop_wl_shell_surface_protocol_set_popup(struct wl_client *wl_client,

struct weston_desktop_surface *dsurface =
wl_resource_get_user_data(resource);
struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
-   struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
+   struct weston_desktop_seat *seat;
struct weston_surface *parent =
wl_resource_get_user_data(parent_resource);
struct weston_desktop_surface *parent_surface;
struct weston_desktop_wl_shell_surface *surface =
weston_desktop_surface_get_implementation_data(dsurface);
  
+	if (wseat == NULL)

+   return;


See comment on v6.



+
+   seat = weston_desktop_seat_from_seat(wseat);
if (seat == NULL) {
wl_client_post_no_memory(wl_client);
return;
diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
index ebe7940e..ac3de78a 100644
--- a/libweston-desktop/xdg-shell-v5.c
+++ b/libweston-desktop/xdg-shell-v5.c
@@ -421,6 +421,9 @@ weston_desktop_xdg_surface_protocol_show_window_menu(struct 
wl_client *wl_client
struct weston_desktop_xdg_surface *surface =
weston_desktop_surface_get_implementation_data(dsurface);
  
+	if (seat == NULL)

+   return;
+
weston_desktop_xdg_surface_ensure_added(surface);
weston_desktop_api_show_window_menu(surface->desktop, dsurface, seat, 
x, y);
  }
@@ -438,6 +441,9 @@

Re: [PATCH weston v2 3/7] libweston: Make weston_seat release safe

2018-02-13 Thread Pekka Paalanen 
On Thu,  8 Feb 2018 15:37:54 +0200
Alexandros Frantzis  wrote:

> Ensure the server can safely handle client requests for wl_seat resource
> that have become inert due to weston_seat object release and subsequent
> destruction.
> 
> The clean-up involves, among other things, unsetting the destroyed
> weston_seat object from the user data of wl_seat resources, and handling
> this NULL user data case where required.
> 
> The list of sites extracting and using weston_seat object from wl_seat
> resources which were audited for this patch are:
> 
> Legend:
> N/A = Not Applicable (not implemented by weston)
> FIXED = Fixed in the commit
> OK = Already works correctly
> 
> == keyboard_shortcuts_inhibit_unstable_v1 ==
> [N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
> == tablet_input_unstable_v{1,2} ==
> [N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
> == text_input_unstable_v1 ==
> [FIXED] zwp_text_input_v1.activate
> [FIXED] zwp_text_input_v1.deactivate
> == wl_data_device ==
> [FIXED] wl_data_device_manager.get_data_device
> [OK] wl_data_device.start_drag
> [FIXED] wl_data_device.set_selection
> [OK] wl_data_device.release
> == wl_shell ==
> [FIXED] wl_shell_surface.move
> [FIXED] wl_shell_surface.resize
> [FIXED] wl_shell_surface.set_popup
> == xdg_shell and xdg_shell_unstable_v6 ==
> [FIXED] xdg_toplevel.show_window_menu
> [FIXED] xdg_toplevel.move
> [FIXED] xdg_toplevel.resize
> [FIXED] xdg_popup.grab
> == xdg_shell_unstable_v5 ==
> [FIXED] xdg_shell.get_xdg_popup
> [FIXED] xdg_surface.show_window_menu
> [FIXED] xdg_surface.move
> [FIXED] xdg_surface.resize
> 
> Signed-off-by: Alexandros Frantzis 
> ---
> Changes in v2:
>  - Properly create inert resources in seat_get_pointer/touch/keyboard.
>  - Ensure all sites which have a wl_seat input resource can deal
>with inert resources.
> 
>  compositor/text-backend.c|  8 --
>  libweston-desktop/wl-shell.c | 12 +++-
>  libweston-desktop/xdg-shell-v5.c | 16 ++-
>  libweston-desktop/xdg-shell-v6.c | 18 +++-
>  libweston/data-device.c  | 15 ++
>  libweston/input.c| 61 
> 
>  6 files changed, 102 insertions(+), 28 deletions(-)

Hi Alf,

awesome work!

The changes look good to me, however I did not review the
libweston-desktop bits yet. I'm hoping Quentin would take a look at
those.

Therefore, apart from libweston-desktop bits, this patch is:
Reviewed-by: Pekka Paalanen 


Thanks,
pq

> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
> index 66553f45..3386d48b 100644
> --- a/libweston-desktop/wl-shell.c
> +++ b/libweston-desktop/wl-shell.c
> @@ -220,6 +220,9 @@ weston_desktop_wl_shell_surface_protocol_move(struct 
> wl_client *wl_client,
>   struct weston_desktop_wl_shell_surface *surface =
>   weston_desktop_surface_get_implementation_data(dsurface);
>  
> + if (seat == NULL)
> + return;
> +
>   weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
>  }
>  
> @@ -238,6 +241,9 @@ weston_desktop_wl_shell_surface_protocol_resize(struct 
> wl_client *wl_client,
>   enum weston_desktop_surface_edge surf_edges =
>   (enum weston_desktop_surface_edge) edges;
>  
> + if (seat == NULL)
> + return;
> +
>   weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, 
> surf_edges);
>  }
>  
> @@ -321,13 +327,17 @@ 
> weston_desktop_wl_shell_surface_protocol_set_popup(struct wl_client 
> *wl_client,
>   struct weston_desktop_surface *dsurface =
>   wl_resource_get_user_data(resource);
>   struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
> - struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
> + struct weston_desktop_seat *seat;
>   struct weston_surface *parent =
>   wl_resource_get_user_data(parent_resource);
>   struct weston_desktop_surface *parent_surface;
>   struct weston_desktop_wl_shell_surface *surface =
>   weston_desktop_surface_get_implementation_data(dsurface);
>  
> + if (wseat == NULL)
> + return;
> +
> + seat = weston_desktop_seat_from_seat(wseat);
>   if (seat == NULL) {
>   wl_client_post_no_memory(wl_client);
>   return;
> diff --git a/libweston-desktop/xdg-shell-v5.c 
> b/libweston-desktop/xdg-shell-v5.c
> index ebe7940e..ac3de78a 100644
> --- a/libweston-desktop/xdg-shell-v5.c
> +++ b/libweston-desktop/xdg-shell-v5.c
> @@ -421,6 +421,9 @@ 
> weston_desktop_xdg_surface_protocol_show_window_menu(struct wl_client 
> *wl_client
>   struct weston_desktop_xdg_surface *surface =
>   weston_desktop_surface_get_implementation_data(dsurface);
>  
> + if (seat == NULL)
> + return;
> +
>

[PATCH weston v2 3/7] libweston: Make weston_seat release safe

2018-02-08 Thread Alexandros Frantzis 
Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.

The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.

The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:

Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly

== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize

Signed-off-by: Alexandros Frantzis 
---
Changes in v2:
 - Properly create inert resources in seat_get_pointer/touch/keyboard.
 - Ensure all sites which have a wl_seat input resource can deal
   with inert resources.

 compositor/text-backend.c|  8 --
 libweston-desktop/wl-shell.c | 12 +++-
 libweston-desktop/xdg-shell-v5.c | 16 ++-
 libweston-desktop/xdg-shell-v6.c | 18 +++-
 libweston/data-device.c  | 15 ++
 libweston/input.c| 61 
 6 files changed, 102 insertions(+), 28 deletions(-)

diff --git a/compositor/text-backend.c b/compositor/text-backend.c
index e6ee249c..4d8c085b 100644
--- a/compositor/text-backend.c
+++ b/compositor/text-backend.c
@@ -193,10 +193,14 @@ text_input_activate(struct wl_client *client,
 {
struct text_input *text_input = wl_resource_get_user_data(resource);
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
-   struct input_method *input_method = weston_seat->input_method;
+   struct input_method *input_method;
struct weston_compositor *ec = text_input->ec;
struct text_input *current;
 
+   if (!weston_seat)
+   return;
+
+   input_method = weston_seat->input_method;
if (input_method->input == text_input)
return;
 
@@ -237,7 +241,7 @@ text_input_deactivate(struct wl_client *client,
 {
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
 
-   if (weston_seat->input_method->input)
+   if (weston_seat && weston_seat->input_method->input)
deactivate_input_method(weston_seat->input_method);
 }
 
diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
index 66553f45..3386d48b 100644
--- a/libweston-desktop/wl-shell.c
+++ b/libweston-desktop/wl-shell.c
@@ -220,6 +220,9 @@ weston_desktop_wl_shell_surface_protocol_move(struct 
wl_client *wl_client,
struct weston_desktop_wl_shell_surface *surface =
weston_desktop_surface_get_implementation_data(dsurface);
 
+   if (seat == NULL)
+   return;
+
weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
 }
 
@@ -238,6 +241,9 @@ weston_desktop_wl_shell_surface_protocol_resize(struct 
wl_client *wl_client,
enum weston_desktop_surface_edge surf_edges =
(enum weston_desktop_surface_edge) edges;
 
+   if (seat == NULL)
+   return;
+
weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, 
surf_edges);
 }
 
@@ -321,13 +327,17 @@ weston_desktop_wl_shell_surface_protocol_set_popup(struct 
wl_client *wl_client,
struct weston_desktop_surface *dsurface =
wl_resource_get_user_data(resource);
struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
-   struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
+   struct weston_desktop_seat *seat;
struct weston_surface *parent =
wl_resource_get_user_data(parent_resource);
struct weston_desktop_surface *parent_surface;
struct weston_desktop_wl_shell_surface *surface =
weston_desktop_surface_get_implementation_data(dsurface);
 
+   if (wseat == NULL)
+   return;
+
+   seat = weston_desktop_seat_from_seat(wseat);
if (seat == NULL) {
wl_client_post_no_memory(wl_client);
return;
diff --git