Re: [Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for embedded bytecode loader.

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

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@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