[PATCH v2] xwm: Fix memory leak

[Derek Foreman]

On 2018-03-20 10:26 AM, Scott Moreau wrote:
> A memory leak introduced by 6b58ea8c led to me finding a bigger
> leak, which is xwm was calling frame_create() without calling
> frame_destroy(). This meant that the associated icon_surface
> was not being destroyed, leaving the destroy handler for it
> broken. Here we fix this by calling frame_destroy() when the
> window is destroyed and free the reply in the icon_surface
> destroy handler.

Ran valgrind memcheck to try to confirm this resolved a leak, found a
lot more leaks. ugh. :)

> ---
>  xwayland/window-manager.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..9e61a67 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -1353,6 +1353,14 @@ weston_wm_window_schedule_repaint(struct weston_wm_window *window)
>  }
>  
>  static void
> +handle_icon_surface_destroy(void *data)
> +{
> +	xcb_get_property_reply_t *reply = (xcb_get_property_reply_t *)data;
> +
> +	free(reply);

free takes a void * anyway, so just free(data); should be fine.

> +}
> +
> +static void
>  weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  {
>  	xcb_get_property_reply_t *reply;
> @@ -1371,16 +1379,20 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  	length = xcb_get_property_value_length(reply);
>  
>  	/* This is in 32-bit words, not in bytes. */
> -	if (length < 2)
> +	if (length < 2) {
> +		free(reply);
>  		return;
> +	}
>  
>  	data = xcb_get_property_value(reply);
>  	width = *data++;
>  	height = *data++;
>  
>  	/* Some checks against malformed input. */
> -	if (width == 0 || height == 0 || length < 2 + width * height)
> +	if (width == 0 || height == 0 || length < 2 + width * height) {
> +		free(reply);
>  		return;
> +	}
>  
>  	new_surface =
>  		cairo_image_surface_create_for_data((unsigned char *)data,
> @@ -1390,9 +1402,13 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  	/* Bail out in case anything wrong happened during surface creation. */
>  	if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
>  		cairo_surface_destroy(new_surface);
> +		free(reply);
>  		return;
>  	}
>  
> +	cairo_surface_set_user_data(new_surface, NULL, reply,

(wrong tab size again)

Those nits resolved and this is
Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

Thanks,
Derek

> +								&handle_icon_surface_destroy);
> +
>  	if (window->frame)
>  		frame_set_icon(window->frame, new_surface);
>  	else /* We don’t have a frame yet */
> @@ -1502,6 +1518,9 @@ weston_wm_window_destroy(struct weston_wm_window *window)
>  		window->frame_id = XCB_WINDOW_NONE;
>  	}
>  
> +	if (window->frame)
> +		frame_destroy(window->frame);
> +
>  	if (window->surface_id)
>  		wl_list_remove(&window->link);
>  
>