From e57af377bb8cd032f19df9f657fdd7a064273ced Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Mon, 28 May 2012 09:29:47 +0200 Subject: [PATCH] Don't pass cairo surfaces around on the lua stack Now that the C code uses lightuserdata for passing around cairo surfaces, they are no longer automatically garbage collected. To avoid memleaks, this commit compares the C code to use cairo_surface_t pointers instead of the lua stack. This also fixes a memleak were a client's icon was leaked. Signed-off-by: Uli Schlachter --- draw.c | 17 +++++++---------- draw.h | 4 ++-- ewmh.c | 10 +++++----- ewmh.h | 4 +++- luaa.c | 7 ++++++- objects/client.c | 34 ++++++++++++++++++++-------------- objects/client.h | 2 +- property.c | 14 +++++--------- 8 files changed, 49 insertions(+), 43 deletions(-) diff --git a/draw.c b/draw.c index dc8a60db2..7f0bfa1dd 100644 --- a/draw.c +++ b/draw.c @@ -105,8 +105,8 @@ free_data(void *data) * \param data The image's data in ARGB format, will be copied by this function. * \return Number of items pushed on the lua stack. */ -int -luaA_surface_from_data(lua_State *L, int width, int height, uint32_t *data) +cairo_surface_t * +draw_surface_from_data(int width, int height, uint32_t *data) { unsigned long int len = width * height; unsigned long int i; @@ -133,10 +133,7 @@ luaA_surface_from_data(lua_State *L, int width, int height, uint32_t *data) /* This makes sure that buffer will be freed */ cairo_surface_set_user_data(surface, &data_key, buffer, &free_data); - /* lua has to make sure to free the ref or we have a leak */ - lua_pushlightuserdata(L, surface); - - return 1; + return surface; } /** Duplicate the specified image surface. @@ -205,21 +202,21 @@ image_imlib_load_strerror(Imlib_Load_Error e) * \param path file to load * \return A cairo image surface or NULL on error. */ -int +cairo_surface_t * draw_load_image(lua_State *L, const char *path) { Imlib_Image imimage; Imlib_Load_Error e = IMLIB_LOAD_ERROR_NONE; - int ret; + cairo_surface_t *ret; imimage = imlib_load_image_with_error_return(path, &e); if (!imimage) { luaL_error(L, "Cannot load image '%s': %s", path, image_imlib_load_strerror(e)); - return 0; + return NULL; } imlib_context_set_image(imimage); - ret = luaA_surface_from_data(L, imlib_image_get_width(), + ret = draw_surface_from_data(imlib_image_get_width(), imlib_image_get_height(), imlib_image_get_data_for_reading_only()); imlib_free_image_and_decache(); return ret; diff --git a/draw.h b/draw.h index c87db2d5e..bc7720359 100644 --- a/draw.h +++ b/draw.h @@ -65,9 +65,9 @@ a_iso2utf8(const char *str, ssize_t len, char **dest, ssize_t *dlen) return false; } -int luaA_surface_from_data(lua_State *L, int width, int height, uint32_t *data); +cairo_surface_t *draw_surface_from_data(int width, int height, uint32_t *data); cairo_surface_t *draw_dup_image_surface(cairo_surface_t *surface); -int draw_load_image(lua_State *L, const char *path); +cairo_surface_t *draw_load_image(lua_State *L, const char *path); #endif // vim: filetype=c:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 diff --git a/ewmh.c b/ewmh.c index d0bd0ce60..3bd94f534 100644 --- a/ewmh.c +++ b/ewmh.c @@ -634,7 +634,7 @@ ewmh_window_icon_get_unchecked(xcb_window_t w) _NET_WM_ICON, XCB_ATOM_CARDINAL, 0, UINT32_MAX); } -static int +static cairo_surface_t * ewmh_window_icon_from_reply(xcb_get_property_reply_t *r) { uint32_t *data; @@ -655,20 +655,20 @@ ewmh_window_icon_from_reply(xcb_get_property_reply_t *r) if (!data[0] || !data[1] || len > r->length - 2) return 0; - return luaA_surface_from_data(globalconf.L, data[0], data[1], data + 2); + return draw_surface_from_data(data[0], data[1], data + 2); } /** Get NET_WM_ICON. * \param cookie The cookie. * \return The number of elements on stack. */ -int +cairo_surface_t * ewmh_window_icon_get_reply(xcb_get_property_cookie_t cookie) { xcb_get_property_reply_t *r = xcb_get_property_reply(globalconf.connection, cookie, NULL); - int ret = ewmh_window_icon_from_reply(r); + cairo_surface_t *surface = ewmh_window_icon_from_reply(r); p_delete(&r); - return ret; + return surface; } // vim: filetype=c:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 diff --git a/ewmh.h b/ewmh.h index d7f27d0e4..4a8c0833e 100644 --- a/ewmh.h +++ b/ewmh.h @@ -22,6 +22,8 @@ #ifndef AWESOME_EWMH_H #define AWESOME_EWMH_H +#include + #include "globalconf.h" #include "strut.h" @@ -37,7 +39,7 @@ void ewmh_process_client_strut(client_t *); void ewmh_update_strut(xcb_window_t, strut_t *); void ewmh_update_window_type(xcb_window_t window, uint32_t type); xcb_get_property_cookie_t ewmh_window_icon_get_unchecked(xcb_window_t); -int ewmh_window_icon_get_reply(xcb_get_property_cookie_t); +cairo_surface_t *ewmh_window_icon_get_reply(xcb_get_property_cookie_t); #endif // vim: filetype=c:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 diff --git a/luaa.c b/luaa.c index 6fe8c1cfe..30f29983d 100644 --- a/luaa.c +++ b/luaa.c @@ -109,7 +109,12 @@ static int luaA_load_image(lua_State *L) { const char *filename = luaL_checkstring(L, 1); - return draw_load_image(L, filename); + cairo_surface_t *surface = draw_load_image(L, filename); + if (!surface) + return 0; + /* lua has to make sure to free the ref or we have a leak */ + lua_pushlightuserdata(L, surface); + return 1; } /** UTF-8 aware string length computing. diff --git a/objects/client.c b/objects/client.c index 237ce24a1..1fbbbf112 100644 --- a/objects/client.c +++ b/objects/client.c @@ -1040,19 +1040,17 @@ luaA_client_isvisible(lua_State *L) * \param iidx The image index on the stack. */ void -client_set_icon(lua_State *L, int cidx, int iidx) +client_set_icon(client_t *c, cairo_surface_t *s) { - client_t *c = luaA_checkudata(L, cidx, &client_class); - cairo_surface_t *surf = NULL; - if(!lua_isnil(L, iidx)) - { - cairo_surface_t **cairo_surface = (cairo_surface_t **)lua_touserdata(L, iidx); - surf = draw_dup_image_surface(*cairo_surface); - } + if (s) + s = draw_dup_image_surface(s); if(c->icon) cairo_surface_destroy(c->icon); - c->icon = surf; - luaA_object_emit_signal(L, cidx < iidx ? cidx : cidx - 1, "property::icon", 0); + c->icon = s; + + luaA_object_push(globalconf.L, c); + luaA_object_emit_signal(globalconf.L, -1, "property::icon", 0); + lua_pop(globalconf.L, 1); } /** Kill a client. @@ -1294,7 +1292,10 @@ luaA_client_set_maximized_vertical(lua_State *L, client_t *c) static int luaA_client_set_icon(lua_State *L, client_t *c) { - client_set_icon(L, -3, -1); + cairo_surface_t *surf = NULL; + if(!lua_isnil(L, -1)) + surf = (cairo_surface_t *)lua_touserdata(L, -1); + client_set_icon(c, surf); return 0; } @@ -1393,7 +1394,7 @@ luaA_client_get_content(lua_State *L, client_t *c) c->geometry.width, c->geometry.height, ~0, XCB_IMAGE_FORMAT_Z_PIXMAP); - int retval = 0; + cairo_surface_t *surface = NULL; if(ximage) { @@ -1408,13 +1409,18 @@ luaA_client_get_content(lua_State *L, client_t *c) data[y * ximage->width + x] |= 0xff000000; /* set alpha to 0xff */ } - retval = luaA_surface_from_data(L, ximage->width, ximage->height, data); + surface = draw_surface_from_data(ximage->width, ximage->height, data); p_delete(&data); } xcb_image_destroy(ximage); } - return retval; + if (!surface) + return 0; + + /* lua has to make sure to free the ref or we have a leak */ + lua_pushlightuserdata(L, surface); + return 1; } static int diff --git a/objects/client.h b/objects/client.h index 0f06af4ba..4846b4f0a 100644 --- a/objects/client.h +++ b/objects/client.h @@ -142,7 +142,7 @@ void client_set_transient_for(lua_State *L, int, client_t *); void client_set_name(lua_State *L, int, char *); void client_set_alt_name(lua_State *L, int, char *); void client_set_group_window(lua_State *, int, xcb_window_t); -void client_set_icon(lua_State *, int, int); +void client_set_icon(client_t *c, cairo_surface_t *s); void client_set_skip_taskbar(lua_State *, int, bool); void client_focus(client_t *); void client_focus_update(client_t *); diff --git a/property.c b/property.c index 610174628..51eaaec1a 100644 --- a/property.c +++ b/property.c @@ -241,17 +241,13 @@ property_get_net_wm_icon(client_t *c) void property_update_net_wm_icon(client_t *c, xcb_get_property_cookie_t cookie) { - luaA_object_push(globalconf.L, c); + cairo_surface_t *surface = ewmh_window_icon_get_reply(cookie); - if(ewmh_window_icon_get_reply(cookie)) - { - client_set_icon(globalconf.L, -2, -1); - /* remove icon */ - lua_pop(globalconf.L, 1); - } + if(!surface) + return; - /* remove client */ - lua_pop(globalconf.L, 1); + client_set_icon(c, surface); + cairo_surface_destroy(surface); } xcb_get_property_cookie_t