From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 1 Mar 2019 07:04:17 +0300 From: Alexander Turenko Subject: Re: [PATCH v2 2/6] Add functions to ease using Lua iterators from C Message-ID: <20190301040416.kv27ofxwbrlk6ybt@tkn_work_nb> References: <2c8049118f86934d6088636dba44f92f381ef8ff.1547064388.git.alexander.turenko@tarantool.org> <20190110122909.novsa6o6duykot6a@esperanza> <20190115232623.huxpqj4adoqqi3mo@tkn_work_nb> <20190116081802.wpnbjaagm7qftlnb@esperanza> <20190128181716.rqcz4ydjs2krm2hr@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190128181716.rqcz4ydjs2krm2hr@tkn_work_nb> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: Made several minor updates. I'll squash them into the original commit, but show here separatelly to ease reading. WBR, Alexander Turenko. On Mon, Jan 28, 2019 at 09:17:16PM +0300, Alexander Turenko wrote: > Updated a bit: > > diff --git a/src/lua/utils.c b/src/lua/utils.c > index 173d59a59..c8cfa70b3 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -981,6 +981,11 @@ struct luaL_iterator * > luaL_iterator_new(lua_State *L, int idx) > { > struct luaL_iterator *it = malloc(sizeof(struct luaL_iterator)); > + if (it == NULL) { > + diag_set(OutOfMemory, sizeof(struct luaL_iterator), > + "malloc", "luaL_iterator"); > + return NULL; > + } > > if (idx == 0) { > /* gen, param, state are on top of a Lua stack. */ commit c7cf235b5ba68e911a1fd47e950fe205e963f9e7 Author: Alexander Turenko Date: Thu Feb 21 14:06:39 2019 +0300 FIXUP: luaL_iterator: use global Lua state for unreferencing diff --git a/src/lua/utils.c b/src/lua/utils.c index 002c5e4b9..2e5096626 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -1052,11 +1052,11 @@ luaL_iterator_next(lua_State *L, struct luaL_iterator *it) return nresults; } -void luaL_iterator_delete(lua_State *L, struct luaL_iterator *it) +void luaL_iterator_delete(struct luaL_iterator *it) { - luaL_unref(L, LUA_REGISTRYINDEX, it->gen); - luaL_unref(L, LUA_REGISTRYINDEX, it->param); - luaL_unref(L, LUA_REGISTRYINDEX, it->state); + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, it->gen); + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, it->param); + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, it->state); free(it); } diff --git a/src/lua/utils.h b/src/lua/utils.h index 772b5d877..c66a8a4b9 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -562,7 +562,7 @@ luaL_iterator_next(lua_State *L, struct luaL_iterator *it); /** * Free all resources held by the iterator. */ -void luaL_iterator_delete(lua_State *L, struct luaL_iterator *it); +void luaL_iterator_delete(struct luaL_iterator *it); /* }}} */ diff --git a/test/unit/luaL_iterator.c b/test/unit/luaL_iterator.c index 5a254f27d..038d34c5c 100644 --- a/test/unit/luaL_iterator.c +++ b/test/unit/luaL_iterator.c @@ -98,6 +98,13 @@ main() struct lua_State *L = luaL_newstate(); luaL_openlibs(L); + tarantool_L = L; + + /* + * Check that everything works fine in a thread (a fiber) + * other then the main one. + */ + L = lua_newthread(L); /* * Expose luafun. @@ -148,7 +155,7 @@ main() is(lua_gettop(L) - top, 0, "%s: stack size", description); /* Free the luaL_iterator structure. */ - luaL_iterator_delete(L, it); + luaL_iterator_delete(it); /* Check stack size. */ is(lua_gettop(L) - top, 0, "%s: stack size", description); commit 1318e30c89efbb58de46fb5ed167cd61e3ff661a Author: Alexander Turenko Date: Mon Feb 25 01:05:01 2019 +0300 FIXUP: luaL_iterator: tweak a comment diff --git a/src/lua/utils.h b/src/lua/utils.h index c66a8a4b9..c4a16704f 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -543,7 +543,7 @@ struct luaL_iterator; * Create a Lua iterator from a gen, param, state triplet. * * If idx == 0, then three top stack values are used as the - * triplet. + * triplet. Note: they are not popped. * * Otherwise idx is index on Lua stack points to a * {gen, param, state} table. commit f95e07b340f4b698c2d6aa0ff6db437cc388ab85 Author: Alexander Turenko Date: Mon Feb 25 09:50:34 2019 +0300 FIXUP: luaL_iterator: use pcall diff --git a/src/lua/utils.c b/src/lua/utils.c index 2e5096626..94f8c65d6 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -1029,7 +1029,15 @@ luaL_iterator_next(lua_State *L, struct luaL_iterator *it) lua_rawgeti(L, LUA_REGISTRYINDEX, it->gen); lua_rawgeti(L, LUA_REGISTRYINDEX, it->param); lua_rawgeti(L, LUA_REGISTRYINDEX, it->state); - lua_call(L, 2, LUA_MULTRET); + if (luaT_call(L, 2, LUA_MULTRET) != 0) { + /* + * Pop garbage from the call (a gen function + * likely will not leave the stack even when raise + * an error), pop a returned error. + */ + lua_settop(L, frame_start); + return -1; + } int nresults = lua_gettop(L) - frame_start; /* diff --git a/src/lua/utils.h b/src/lua/utils.h index c4a16704f..4a87dac45 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -553,8 +553,11 @@ luaL_iterator_new(lua_State *L, int idx); /** * Move iterator to the next value. Push values returned by - * gen(param, state) and return its count. Zero means no more - * results available. + * gen(param, state). + * + * Return count of pushed values. Zero means no more results + * available. In case of a Lua error in a gen function return -1 + * and set a diag. */ int luaL_iterator_next(lua_State *L, struct luaL_iterator *it); diff --git a/test/unit/luaL_iterator.c b/test/unit/luaL_iterator.c index 038d34c5c..8d25a0062 100644 --- a/test/unit/luaL_iterator.c +++ b/test/unit/luaL_iterator.c @@ -2,7 +2,12 @@ #include /* luaL_*() */ #include /* luaL_openlibs() */ #include "unit.h" /* plan, header, footer, is */ +#include "memory.h" /* memory_init() */ +#include "fiber.h" /* fiber_init() */ +#include "diag.h" /* struct error, diag_*() */ +#include "exception.h" /* type_LuajitError */ #include "lua/utils.h" /* luaL_iterator_*() */ +#include "lua/error.h" /* tarantool_lua_error_init() */ extern char fun_lua[]; @@ -31,6 +36,8 @@ main() int idx; /* How much values are in the iterator. */ int iterations; + /* Expected error (if any). */ + const char *exp_err; } cases[] = { { .description = "pairs, zero idx", @@ -39,6 +46,7 @@ main() .first_value = 42, .idx = 0, .iterations = 1, + .exp_err = NULL, }, { .description = "ipairs, zero idx", @@ -47,6 +55,7 @@ main() .first_value = 42, .idx = 0, .iterations = 3, + .exp_err = NULL, }, { .description = "luafun iterator, zero idx", @@ -55,6 +64,7 @@ main() .first_value = 42, .idx = 0, .iterations = 3, + .exp_err = NULL, }, { .description = "pairs, from table", @@ -63,6 +73,7 @@ main() .first_value = 42, .idx = -1, .iterations = 1, + .exp_err = NULL, }, { .description = "ipairs, from table", @@ -71,6 +82,7 @@ main() .first_value = 42, .idx = -1, .iterations = 3, + .exp_err = NULL, }, { .description = "luafun iterator, from table", @@ -79,19 +91,34 @@ main() .first_value = 42, .idx = -1, .iterations = 3, + .exp_err = NULL, + }, + { + .description = "lua error", + .init = "return error, 'I am the error', 0", + .init_retvals = 3, + .first_value = 0, + .idx = 0, + .iterations = 0, + .exp_err = "I am the error", }, }; int cases_cnt = (int) (sizeof(cases) / sizeof(cases[0])); /* - * * Check stack size after creating luaL_iterator (triple - * times). * * 4 checks per iteration. - * * Check that values ends. + * * 3 checks of a stack size. + * * 1 check that values ends (for success cases). + * * 1 check for an iterator error (for error cases). + * * 1 check for an error type (for error cases). + * * 1 check for an error message (for error cases). */ int planned = 0; - for (int i = 0; i < cases_cnt; ++i) + for (int i = 0; i < cases_cnt; ++i) { planned += cases[i].iterations * 4 + 4; + if (cases[i].exp_err != NULL) + planned += 2; + } plan(planned); header(); @@ -100,6 +127,10 @@ main() luaL_openlibs(L); tarantool_L = L; + memory_init(); + fiber_init(fiber_c_invoke); + tarantool_lua_error_init(L); + /* * Check that everything works fine in a thread (a fiber) * other then the main one. @@ -147,9 +178,20 @@ main() description, j); } - /* Check the iterator ends when expected. */ - int rc = luaL_iterator_next(L, it); - is(rc, 0, "%s: iterator ends", description); + if (cases[i].exp_err == NULL) { + /* Check the iterator ends when expected. */ + int rc = luaL_iterator_next(L, it); + is(rc, 0, "%s: iterator ends", description); + } else { + /* Check expected error. */ + int rc = luaL_iterator_next(L, it); + is(rc, -1, "%s: iterator error", description); + struct error *e = diag_last_error(diag_get()); + is(e->type, &type_LuajitError, "%s: check error type", + description); + ok(!strcmp(e->errmsg, cases[i].exp_err), + "%s: check error message", description); + } /* Check stack size. */ is(lua_gettop(L) - top, 0, "%s: stack size", description); diff --git a/test/unit/luaL_iterator.result b/test/unit/luaL_iterator.result index f4eda5695..2472eedcf 100644 --- a/test/unit/luaL_iterator.result +++ b/test/unit/luaL_iterator.result @@ -1,4 +1,4 @@ -1..80 +1..86 *** main *** ok 1 - pairs, zero idx: stack size ok 2 - pairs, zero idx: iter 0: gen() retval count @@ -80,4 +80,10 @@ ok 77 - luafun iterator, from table: iter: 2: stack size ok 78 - luafun iterator, from table: iterator ends ok 79 - luafun iterator, from table: stack size ok 80 - luafun iterator, from table: stack size +ok 81 - lua error: stack size +ok 82 - lua error: iterator error +ok 83 - lua error: check error type +ok 84 - lua error: check error message +ok 85 - lua error: stack size +ok 86 - lua error: stack size *** main: done *** commit 6edef55a8f689f595a760a9c06180de7da37976e Author: Alexander Turenko Date: Fri Mar 1 02:27:25 2019 +0300 FIXUP: luaL_iterator: after rebase fix diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 263856a7a..663ed2ec2 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -139,9 +139,9 @@ target_link_libraries(histogram.test stat unit) add_executable(ratelimit.test ratelimit.c) target_link_libraries(ratelimit.test unit) add_executable(luaL_iterator.test luaL_iterator.c) -target_link_libraries(luaL_iterator.test unit server core misc +target_link_libraries(luaL_iterator.test unit server coll core misc ${CURL_LIBRARIES} ${LIBYAML_LIBRARIES} ${READLINE_LIBRARIES} - ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES}) + ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES} dl) add_executable(say.test say.c) target_link_libraries(say.test core unit)