Hi, Sergey! Thanks for the review! Fixed nits you mentioned, the branch is force-pushed. -- Best regards, Maxim Kokryashkin     >Пятница, 17 марта 2023, 14:21 +03:00 от Sergey Kaplun : >  >Hi, Maksim! >Thanks for the patch! >LGTM, except a few minor nits regarding the commit message and the >comment. > >On 16.03.23, Maksim Kokryashkin wrote: >> From: Mike Pall >> >> (cherry-picked from commit 90e65514dda3994253c1e3007f63da7ace8f6b7b) >> >> C library loader uses `dlopen` under the hood, which fails, if > >Typo: s/C library loader/The C library loader/ > >> provided library path is longer than PATH_MAX. PATH_MAX is >> 4096 bytes by default, so a corresponsing check is added to > >Typo: s/corresponsing/corresponding/ > >> `ll_loadfunc`. >> >> Maxim Kokryashkin: >> * added the description and the test for the problem >> >> Part of tarantool/tarantool#8069 >> --- >> Side note: Still no adequate constants like PATH_MAX... >> Side note: There is no test for successfull loadlib, since >> there is one in the PUC-Rio suite. >> >> Branch: https://github.com/tarantool/luajit/tree/fckxorg/c-library-path-length >> PR: https://github.com/tarantool/tarantool/pull/8449 >> >> src/lib_package.c | 7 ++++++- >> test/tarantool-tests/c-library-path-length.test.lua | 13 +++++++++++++ >> 2 files changed, 19 insertions(+), 1 deletion(-) >> create mode 100644 test/tarantool-tests/c-library-path-length.test.lua >> >> diff --git a/src/lib_package.c b/src/lib_package.c >> index 8573b9f9..67959a10 100644 >> --- a/src/lib_package.c >> +++ b/src/lib_package.c >> @@ -215,7 +215,12 @@ static const char *mksymname(lua_State *L, const char *modname, >> >> static int ll_loadfunc(lua_State *L, const char *path, const char *name, int r) >> { >> - void **reg = ll_register(L, path); >> + void **reg; >> + if (strlen(path) >= 4096) { > >Side note: Why don't just use PATH_MAX here? AFAIK, it may be less than >4096 bytes. Just complaining. > >> + lua_pushliteral(L, "path too long"); >> + return PACKAGE_ERR_LIB; >> + } >> + reg = ll_register(L, path); >> if (*reg == NULL) *reg = ll_load(L, path, (*name == '*')); >> if (*reg == NULL) { >> return PACKAGE_ERR_LIB; /* Unable to load library. */ >> diff --git a/test/tarantool-tests/c-library-path-length.test.lua b/test/tarantool-tests/c-library-path-length.test.lua >> new file mode 100644 >> index 00000000..11dd0cf4 >> --- /dev/null >> +++ b/test/tarantool-tests/c-library-path-length.test.lua >> @@ -0,0 +1,13 @@ >> +local tap = require('tap') >> +local test = tap.test('c-library-path-length') >> +test:plan(2) >> + >> +-- It doesn't really matter how long that string is, if it is longer than 4096. > >Minor: comment line is longer than 66 symbols. > >> +local long_path = string.rep('/path', 1024) >> +package.cpath = long_path >> + >> +local res, err = package.loadlib(long_path, 'func') >> +test:ok(res == nil, 'loaded library with a too large path') >> +test:like(err, 'path too long', 'incorrect error') >> + >> +os.exit(test:check() and 0 or 1) >> -- >> 2.37.1 (Apple Git-137.1) >> > >-- >Best regards, >Sergey Kaplun