Понедельник, 11 октября 2021, 17:26 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
 

Hi, Maxim!

Thanks for the patch!

LGTM, except a few comments below.

On 24.09.21, Maxim Kokryashkin wrote:
> Loading bytecode with an extra header (BOM or "#") is disabled
> for security reasons since LuaJIT-2.0.0-beta10.
> For more information see comment for `lj_lex_setup()` in <src/lj_lex.c>.
> Also see commit 53a285c ('Disable

Nit: It is better to use the full commit hash here to avoid clashing.

> loading bytecode with an extra header (BOM or #!).').
> Tests are adapted to LuaJIT behavior.
>
> Closes tarantool/tarantool#5691
> Part of tarantool/tarantool#5845
> Part of tarantool/tarantool#4473

Looks like it should be 5870 instead. 5873 is a unbrella for all tests
activities.

> ---

Also, please attach tarantool branch with CI to check it is OK.

> Issue: https://github.com/tarantool/tarantool/issues/5691
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5691-adapt-bytecode-PUC-Rio
>
> test/PUC-Rio-Lua-5.1-tests/main.lua | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/test/PUC-Rio-Lua-5.1-tests/main.lua b/test/PUC-Rio-Lua-5.1-tests/main.lua
> index 07facc4c..c62950c8 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/main.lua
> +++ b/test/PUC-Rio-Lua-5.1-tests/main.lua
> @@ -172,23 +172,17 @@ prepfile(s)
> prepfile[[#comment in 1st line without \n at the end]]
> RUN("lua %s", prog)
>
> --- FIXME: Loading bytecode with an extra header (BOM or "#")
> +-- Loading bytecode with an extra header (BOM or "#")
> -- is disabled for security reasons since LuaJIT-2.0.0-beta10.
> -- For more information see comment for `lj_lex_setup()`
> -- in <src/lj_lex.c>.
> -- Also see commit 53a285c0c3544ff5dea7c67b741c3c2d06d22b47
> -- (Disable loading bytecode with an extra header (BOM or #!).).
> -- See also https://github.com/tarantool/tarantool/issues/5691.
> --- The test is disabled for LuaJIT.
> -prepfile("#comment with a binary file\n"..string.dump(loadstring("print(1)")))
> --- RUN("lua %s > %s", prog, out)
> --- checkout("1\n")
> -
> -prepfile("#comment with a binary file\r\n"..string.dump(loadstring("print(1)")))
> --- FIXME: Behavior is different for LuaJIT. See the comment above.
> --- The test is disabled for LuaJIT.
> --- RUN("lua %s > %s", prog, out)
> --- checkout("1\n")
> +-- The test is adapted to LuaJIT behavior.
> +prepfile(string.dump(loadstring("print(1)")))
> +RUN("lua %s > %s", prog, out)
> +checkout("1\n")

May be it is better to test `NoRun()` for 1 binary file with a comment at the
first string and `Run()` for another binary file without the comment.

>
> -- close Lua with an open file
> prepfile(string.format([[io.output(%q); io.write('alo')]], out))
> --
> 2.33.0
>

--
Best regards,
Sergey Kaplun