Re: [Tarantool-patches] [PATCH luajit 2/2] Fix last commit.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim!
Thanks for the patch!
LGTM, just a few nits below.

On 27.10.23, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
> 
> Reported by PluMGMK.
> 
> (cherry-picked from commit 224129a8e64bfa219d35cd03055bf03952f167f6)
> 
> The error handler for GC finalizers was not cleared from the

It's not a handler itself, but "_VMEVENTS" table, where the handler is
registered.

> stack after the initialization. This commit adds stack cleanup.
> 
> Maxim Kokryashkin:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
>  src/lib_aux.c                                 |  2 ++
>  .../fix-finalizer-error-handler-init.test.c   | 30 +++++++++++++++++++
>  2 files changed, 32 insertions(+)
>  create mode 100644 test/tarantool-c-tests/fix-finalizer-error-handler-init.test.c
> 
> diff --git a/src/lib_aux.c b/src/lib_aux.c
> index d532a12f..ecdc67f9 100644
> --- a/src/lib_aux.c
> +++ b/src/lib_aux.c

<snipped>

> diff --git a/test/tarantool-c-tests/fix-finalizer-error-handler-init.test.c b/test/tarantool-c-tests/fix-finalizer-error-handler-init.test.c
> new file mode 100644
> index 00000000..44c1d96b
> --- /dev/null
> +++ b/test/tarantool-c-tests/fix-finalizer-error-handler-init.test.c

Please, add "lj-991-" prefix and mention the issue in the comment below.

> @@ -0,0 +1,30 @@
> +#include "lua.h"
> +#include "test.h"
> +#include "utils.h"
> +
> +#include "lj_obj.h"
> +
> +/*
> + * This test demonstrates an uncleared Lua stack after the
> + * initialization of the error handler for GC finalizers.
> + */
> +
> +static int stack_is_clean(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	assert_ptr_equal(L->top, L->base);

I suppose, that there is no need for internals to be used here.
We can just use `lua_gettop(L)` and compare its result to 0.

Side note: Also, this helper may be usefull later. See also the comment
in [1].

> +	return TEST_EXIT_SUCCESS;
> +}
> +
> +int main(void)
> +{
> +	lua_State *L = utils_lua_init();
> +
> +	const struct test_unit tgroup[] = {
> +		test_unit_def(stack_is_clean)
> +	};
> +
> +	const int test_result = test_run_group(tgroup, L);
> +	utils_lua_close(L);
> +	return test_result;
> +}
> -- 
> 2.39.3 (Apple Git-145)
> 

[1]: https://github.com/LuaJIT/LuaJIT/commit/03cd5aa7

-- 
Best regards,
Sergey Kaplun