[Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for embedded bytecode loader.
Sergey Kaplun
skaplun at tarantool.org
Mon Sep 11 12:26:46 MSK 2023
Hi, Sergey!
Thanks for the fixes and clarifications!
LGTM, after answering to my comments below.
On 07.09.23, Sergey Bronnikov wrote:
> Hi, Sergey
>
> On 9/5/23 15:55, Sergey Kaplun wrote:
> > Hi, Sergey!
> > Thanks for the patch!
> > Please, consider my comments below.
> >
> > On 31.08.23, Sergey Bronnikov wrote:
> >> From: Sergey Bronnikov <sergeyb at tarantool.org>
> >>
> >> (cherry-picked from commit e49863eda13d095b1a78fd4ca0fd3a6a9a17d782)
> >>
> >> The patch follows up a previous patch and limits the total size of a
> >> chunk load by `lua_load` with size `LJ_MAX_BUF - 1`.
> >>
> >> Sergey Bronnikov:
> >> * added the description and the test
> >> ---
<snipped>
> >> diff --git a/test/tarantool-c-tests/lj-549-lua_load.test.c b/test/tarantool-c-tests/lj-549-lua_load.test.c
> >> new file mode 100644
> >> index 00000000..9baa7a1a
> >> --- /dev/null
> >> +++ b/test/tarantool-c-tests/lj-549-lua_load.test.c
<snipped>
> >
> >> + * `lex_more`.
> >> + */
> >> + assert_true(res != LUA_ERRMEM);
> > Maybe it's better to use here codition res == LUA_OK?
> >
> >> + lua_settop(L, 0);
> >> +
> >> + return TEST_EXIT_SUCCESS;
> >> + }
<snipped>
> >> + static const char *
> >> + bc_reader_with_eof(lua_State *L, void *data, size_t *size)
> >> + {
> >> + UNUSED(L);
> >> + dt *test_data = (dt *)data;
> >> + if (test_data->state == EMIT_EOF) {
> >> + *size = 0;
> >> + return NULL;
> >> + }
> >> +
> >> + static char *bc_chunk = NULL;
> >> + free(bc_chunk);
> > This free is called only once, when bc_chunk is already NULL.
> > I suggest moving the initialization of the `bc_chunk` to the beginning
> > of the scope and calling `free()` only for the `EMIT_EOF` state (it's
> > also a little bit more readable -- a reader shouldn't remember that
> > `free(NULL)` is OK).
> Updated.
Unfortunately, I don't see these changes on branch [1].
> >
> >> +
<snipped>
> >
> > --
> > Best regards,
> > Sergey Kaplun
[1]: https://github.com/tarantool/luajit/blob/f7e5e8abe396411065f8941d04879577e7fae175/test/tarantool-c-tests/lj-549-lua-load.test.c#L70
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list