<div dir="ltr">Hi Derek,<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 23, 2018 at 12:22 PM, Derek Foreman <span dir="ltr"><<a href="mailto:derekf@osg.samsung.com" target="_blank">derekf@osg.samsung.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 2018-03-20 10:26 AM, Scott Moreau wrote:<br>
> A memory leak introduced by 6b58ea8c led to me finding a bigger<br>
> leak, which is xwm was calling frame_create() without calling<br>
> frame_destroy(). This meant that the associated icon_surface<br>
> was not being destroyed, leaving the destroy handler for it<br>
> broken. Here we fix this by calling frame_destroy() when the<br>
> window is destroyed and free the reply in the icon_surface<br>
> destroy handler.<br>
<br>
</span>Ran valgrind memcheck to try to confirm this resolved a leak, found a<br>
lot more leaks. ugh. :)<br></blockquote><div><br></div><div>Yes, I seem to find more and more too.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> ---<br>
>  xwayland/window-manager.c | 23 +++++++++++++++++++++--<br>
>  1 file changed, 21 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c<br>
> index c307e19..9e61a67 100644<br>
> --- a/xwayland/window-manager.c<br>
> +++ b/xwayland/window-manager.c<br>
> @@ -1353,6 +1353,14 @@ weston_wm_window_schedule_<wbr>repaint(struct weston_wm_window *window)<br>
>  }<br>
><br>
>  static void<br>
> +handle_icon_surface_destroy(<wbr>void *data)<br>
> +{<br>
> +     xcb_get_property_reply_t *reply = (xcb_get_property_reply_t *)data;<br>
> +<br>
> +     free(reply);<br>
<br>
</span>free takes a void * anyway, so just free(data); should be fine.<br></blockquote><div><br></div><div>Ok, great.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="h5"><br>
> +}<br>
> +<br>
> +static void<br>
>  weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)<br>
>  {<br>
>       xcb_get_property_reply_t *reply;<br>
> @@ -1371,16 +1379,20 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)<br>
>       length = xcb_get_property_value_length(<wbr>reply);<br>
><br>
>       /* This is in 32-bit words, not in bytes. */<br>
> -     if (length < 2)<br>
> +     if (length < 2) {<br>
> +             free(reply);<br>
>               return;<br>
> +     }<br>
><br>
>       data = xcb_get_property_value(reply);<br>
>       width = *data++;<br>
>       height = *data++;<br>
><br>
>       /* Some checks against malformed input. */<br>
> -     if (width == 0 || height == 0 || length < 2 + width * height)<br>
> +     if (width == 0 || height == 0 || length < 2 + width * height) {<br>
> +             free(reply);<br>
>               return;<br>
> +     }<br>
><br>
>       new_surface =<br>
>               cairo_image_surface_create_<wbr>for_data((unsigned char *)data,<br>
> @@ -1390,9 +1402,13 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)<br>
>       /* Bail out in case anything wrong happened during surface creation. */<br>
>       if (cairo_surface_status(new_<wbr>surface) != CAIRO_STATUS_SUCCESS) {<br>
>               cairo_surface_destroy(new_<wbr>surface);<br>
> +             free(reply);<br>
>               return;<br>
>       }<br>
><br>
> +     cairo_surface_set_user_data(<wbr>new_surface, NULL, reply,<br>
<br>
</div></div>(wrong tab size again)<br></blockquote><div><br></div><div>My editor was set to 4 wide tabs so this should be fixed now.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Those nits resolved and this is<br>
Reviewed-by: Derek Foreman <<a href="mailto:derekf@osg.samsung.com">derekf@osg.samsung.com</a>><br></blockquote><div><br></div><div>Great, thanks for the review.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
Derek<br></blockquote><div><br></div><div>Thanks,<br></div><div>Scott<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> +                                                             &handle_icon_surface_destroy);<br>
> +<br>
>       if (window->frame)<br>
>               frame_set_icon(window->frame, new_surface);<br>
>       else /* We don’t have a frame yet */<br>
> @@ -1502,6 +1518,9 @@ weston_wm_window_destroy(<wbr>struct weston_wm_window *window)<br>
>               window->frame_id = XCB_WINDOW_NONE;<br>
>       }<br>
><br>
> +     if (window->frame)<br>
> +             frame_destroy(window->frame);<br>
> +<br>
>       if (window->surface_id)<br>
>               wl_list_remove(&window->link);<br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div></div>