[PATCH 1/1] libweston: add weston_view_set_output()

[Fabien Lahoudere]

From: Semi Malinen <semi.malinen at ge.com>

Instead of desktop shell assigning view outputs directly,
use a new method, weston_view_set_output(). The method can
set up an output destroy listener to make sure that views
do not have stale output pointers.

Without this patch it is possible to end up in a scenario
where, e.g. configure_static_view() accesses memory that
has already been freed. The scenario can be provoked by
repeatedly plugging and unplugging a display. The faulty
memory accesses are reported by valgrind.

Signed-off-by: Semi Malinen <semi.malinen at ge.com>
Signed-off-by: Fabien Lahoudere <fabien.lahoudere at collabora.com>
---
 desktop-shell/shell.c  | 11 +++++++----
 libweston/compositor.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 libweston/compositor.h |  4 ++++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..1e153cb 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -580,7 +580,7 @@ create_focus_surface(struct weston_compositor *ec,
 		free(fsurf);
 		return NULL;
 	}
-	fsurf->view->output = output;
+	weston_view_set_output(fsurf->view, output);
 	fsurf->view->is_mapped = true;
 
 	weston_surface_set_size(surface, output->width, output->height);
@@ -2464,7 +2464,7 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
 	shsurf->view->is_mapped = true;
 	if (shsurf->state.maximized) {
 		surface->output = shsurf->output;
-		shsurf->view->output = shsurf->output;
+		weston_view_set_output(shsurf->view, shsurf->output);
 	}
 
 	if (!shell->locked) {
@@ -2881,6 +2881,9 @@ configure_static_view(struct weston_view *ev, struct weston_layer *layer, int x,
 {
 	struct weston_view *v, *next;
 
+	if (!ev->output)
+		return;
+
 	wl_list_for_each_safe(v, next, &layer->view_list.link, layer_link.link) {
 		if (v->output == ev->output && v != ev) {
 			weston_view_unmap(v);
@@ -2970,7 +2973,7 @@ desktop_shell_set_background(struct wl_client *client,
 	surface->committed_private = shell;
 	weston_surface_set_label_func(surface, background_get_label);
 	surface->output = weston_head_from_resource(output_resource)->output;
-	view->output = surface->output;
+	weston_view_set_output(view, surface->output);
 
 	sh_output = find_shell_output_from_weston_output(shell, surface->output);
 	if (sh_output->background_surface) {
@@ -3067,7 +3070,7 @@ desktop_shell_set_panel(struct wl_client *client,
 	surface->committed_private = shell;
 	weston_surface_set_label_func(surface, panel_get_label);
 	surface->output = weston_head_from_resource(output_resource)->output;
-	view->output = surface->output;
+	weston_view_set_output(view, surface->output);
 
 	sh_output = find_shell_output_from_weston_output(shell, surface->output);
 	if (sh_output->panel_surface) {
diff --git a/libweston/compositor.c b/libweston/compositor.c
index dbf61ef..9be66fc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1021,6 +1021,45 @@ weston_surface_update_output_mask(struct weston_surface *es, uint32_t mask)
 	}
 }
 
+static void
+notify_view_output_destroy(struct wl_listener *listener, void *data)
+{
+	struct weston_view *view =
+		container_of(listener,
+		     struct weston_view, output_destroy_listener);
+
+	view->output = NULL;
+	view->output_destroy_listener.notify = NULL;
+}
+
+/** Set the primary output of the view
+ *
+ * \param view The view whose primary output to set
+ * \param output The new primary output for the view
+ *
+ * Set \a output to be the primary output of the \a view.
+ *
+ * Notice that the assignment may be temporary; the primary output could be
+ * automatically changed. Hence, one cannot rely on the value persisting.
+ *
+ * Passing NULL as /a output will set the primary output to NULL.
+ */
+WL_EXPORT void
+weston_view_set_output(struct weston_view *view, struct weston_output *output)
+{
+	if (view->output_destroy_listener.notify) {
+		wl_list_remove(&view->output_destroy_listener.link);
+		view->output_destroy_listener.notify = NULL;
+	}
+	view->output = output;
+	if (output) {
+		view->output_destroy_listener.notify =
+			notify_view_output_destroy;
+		wl_signal_add(&output->destroy_signal,
+			      &view->output_destroy_listener);
+	}
+}
+
 /** Recalculate which output(s) the surface has views displayed on
  *
  * \param es  The surface to remap to outputs
@@ -1112,7 +1151,7 @@ weston_view_assign_output(struct weston_view *ev)
 	}
 	pixman_region32_fini(&region);
 
-	ev->output = new_output;
+	weston_view_set_output(ev, new_output);
 	ev->output_mask = mask;
 
 	weston_surface_assign_output(ev->surface);
@@ -1822,7 +1861,7 @@ weston_view_unmap(struct weston_view *view)
 		return;
 
 	weston_view_damage_below(view);
-	view->output = NULL;
+	weston_view_set_output(view, NULL);
 	view->plane = NULL;
 	view->is_mapped = false;
 	weston_layer_entry_remove(&view->layer_link);
diff --git a/libweston/compositor.h b/libweston/compositor.h
index ca23407..bfad890 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1194,6 +1194,7 @@ struct weston_view {
 	 * view, inheriting the primary output for related views in shells, etc.
 	 */
 	struct weston_output *output;
+	struct wl_listener output_destroy_listener;
 
 	/*
 	 * A more complete representation of all outputs this surface is
@@ -1409,6 +1410,9 @@ void
 weston_version(int *major, int *minor, int *micro);
 
 void
+weston_view_set_output(struct weston_view *view, struct weston_output *output);
+
+void
 weston_view_update_transform(struct weston_view *view);
 
 void
-- 
1.8.3.1