From ff029cb4cfae6e9984821d5f9c9dd49c2d566f96 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sun, 19 Oct 2014 05:46:16 +0200 Subject: lua: strictly free memory on errors Thanks to the recently introduced mp_lua_PITA(), this is "simple" now. It fixes leaks on Lua errors. The hack to avoid stack overflows manually isn't needed anymore, and the Lua error handler will take care of this. --- options/m_option.c | 3 ++- player/client.h | 4 ++++ player/lua.c | 55 ++++++++++++++++++++++++++---------------------------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/options/m_option.c b/options/m_option.c index a84d7e4d41..52a222e87e 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -35,6 +35,7 @@ #include #include "libmpv/client.h" +#include "player/client.h" #include "talloc.h" #include "common/common.h" @@ -3092,7 +3093,7 @@ static void copy_node(const m_option_t *opt, void *dst, const void *src) dup_node(NULL, &VAL(dst)); } -static void *node_get_alloc(struct mpv_node *node) +void *node_get_alloc(struct mpv_node *node) { // Assume it was allocated with copy_node(), which allocates all // sub-nodes with the parent node as talloc parent. diff --git a/player/client.h b/player/client.h index 0cb4fff43f..db248e0cef 100644 --- a/player/client.h +++ b/player/client.h @@ -2,6 +2,7 @@ #define MP_CLIENT_H_ #include +#include #include "libmpv/client.h" @@ -25,4 +26,7 @@ struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name struct mp_log *mp_client_get_log(struct mpv_handle *ctx); struct MPContext *mp_client_get_core(struct mpv_handle *ctx); +// m_option.c +void *node_get_alloc(struct mpv_node *node); + #endif diff --git a/player/lua.c b/player/lua.c index 306f3a66f5..b9ac756bf1 100644 --- a/player/lua.c +++ b/player/lua.c @@ -106,6 +106,9 @@ static int destroy_crap(lua_State *L) // This can be used to free temporary C data structures correctly if Lua errors // happen. // You can't free the talloc context directly; the Lua __gc handler does this. +// In my cases, talloc_free_children(returnval) will be used to free attached +// memory in advance when it's known not to be needed anymore (a minor +// optimization). Freeing it completely must be left to the Lua GC. static void *mp_lua_PITA(lua_State *L) { void **data = lua_newuserdata(L, sizeof(void *)); // u @@ -120,6 +123,12 @@ static void *mp_lua_PITA(lua_State *L) return *data; } +// Perform the equivalent of mpv_free_node_contents(node) when tmp is freed. +static void auto_free_node(void *tmp, mpv_node *node) +{ + talloc_steal(tmp, node_get_alloc(node)); +} + static struct script_ctx *get_ctx(lua_State *L) { lua_getfield(L, LUA_REGISTRYINDEX, "ctx"); @@ -450,7 +459,7 @@ static int script_resume_all(lua_State *L) return 0; } -static bool pushnode(lua_State *L, mpv_node *node, int depth); +static void pushnode(lua_State *L, mpv_node *node); static int script_wait_event(lua_State *L) { @@ -517,8 +526,7 @@ static int script_wait_event(lua_State *L) lua_setfield(L, -2, "name"); switch (prop->format) { case MPV_FORMAT_NODE: - if (!pushnode(L, prop->data, 50)) - luaL_error(L, "stack overflow"); + pushnode(L, prop->data); break; case MPV_FORMAT_DOUBLE: lua_pushnumber(L, *(double *)prop->data); @@ -815,11 +823,8 @@ static int script_get_property_number(lua_State *L) } } -static bool pushnode(lua_State *L, mpv_node *node, int depth) +static void pushnode(lua_State *L, mpv_node *node) { - depth--; - if (depth < 0) - return false; luaL_checkstack(L, 6, "stack overflow"); switch (node->format) { @@ -843,8 +848,7 @@ static bool pushnode(lua_State *L, mpv_node *node, int depth) lua_getfield(L, LUA_REGISTRYINDEX, "ARRAY"); // table mt lua_setmetatable(L, -2); // table for (int n = 0; n < node->u.list->num; n++) { - if (!pushnode(L, &node->u.list->values[n], depth)) // table value - return false; + pushnode(L, &node->u.list->values[n]); // table value lua_rawseti(L, -2, n + 1); // table } break; @@ -854,8 +858,7 @@ static bool pushnode(lua_State *L, mpv_node *node, int depth) lua_setmetatable(L, -2); // table for (int n = 0; n < node->u.list->num; n++) { lua_pushstring(L, node->u.list->keys[n]); // table key - if (!pushnode(L, &node->u.list->values[n], depth)) // table key value - return false; + pushnode(L, &node->u.list->values[n]); // table key value lua_rawset(L, -3); } break; @@ -867,26 +870,24 @@ static bool pushnode(lua_State *L, mpv_node *node, int depth) lua_setmetatable(L, -2); // table break; } - return true; } static int script_get_property_native(lua_State *L) { struct script_ctx *ctx = get_ctx(L); const char *name = luaL_checkstring(L, 1); + void *tmp = mp_lua_PITA(L); mpv_node node; int err = mpv_get_property(ctx->client, name, MPV_FORMAT_NODE, &node); - const char *errstr = mpv_error_string(err); if (err >= 0) { - bool ok = pushnode(L, &node, 50); - mpv_free_node_contents(&node); - if (ok) - return 1; - errstr = "value too large"; + auto_free_node(tmp, &node); + pushnode(L, &node); + talloc_free_children(tmp); + return 1; } lua_pushvalue(L, 2); - lua_pushstring(L, errstr); + lua_pushstring(L, mpv_error_string(err)); return 2; } @@ -931,17 +932,14 @@ static int script_command_native(lua_State *L) void *tmp = mp_lua_PITA(L); makenode(tmp, &node, L, 1); int err = mpv_command_node(ctx->client, &node, &result); - talloc_free_children(tmp); - const char *errstr = mpv_error_string(err); if (err >= 0) { - bool ok = pushnode(L, &result, 50); - mpv_free_node_contents(&result); - if (ok) - return 1; - errstr = "command result too large"; + auto_free_node(tmp, &result); + pushnode(L, &result); + talloc_free_children(tmp); + return 1; } lua_pushvalue(L, 2); - lua_pushstring(L, errstr); + lua_pushstring(L, mpv_error_string(err)); return 2; } @@ -1303,8 +1301,7 @@ static int script_parse_json(lua_State *L) ok = !text[0] || !trail; } if (ok) { - if (!pushnode(L, &node, 64)) - luaL_error(L, "stack overflow"); + pushnode(L, &node); lua_pushnil(L); } else { lua_pushnil(L); -- cgit v1.2.3