

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