<HTML><BODY><div>Hi! Thanks for the review.</div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16890807800533511837_BODY">Hello, Max!<br><br>Thanks for the patch! See my comments.<br><br><br>Sergey<br><br>On 7/10/23 14:24, Maksim Kokryashkin wrote:<br>> From: Maxim Kokryashkin <<a href="/compose?To=m.kokryashkin@tarantool.org">m.kokryashkin@tarantool.org</a>><br>><br>> It is really inconvenient to use a standalone shell script to parse<br>> memprof dump. That is why this commit introduces a CLI flag for tools<br>> to the LuaJIT, so now it is possible to parse memprof dump<br>> as simple as:<br>> ```<br>> luajit -tm memprof.bin<br>> ```<br>><br>> Closes tarantool/tarantool#5688<br>> ---<br>> Changes in v7:<br>> - Fixed comments as per review by Sergey<br>><br>> Branch: <a href="https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse" target="_blank">https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse</a><br>> PR: <a href="https://github.com/tarantool/tarantool/pull/8002" target="_blank">https://github.com/tarantool/tarantool/pull/8002</a><br>><br>> src/luajit.c | 32 +++++++++-<br>> .../gh-5688-memprof-cli-flag.test.lua | 58 +++++++++++++++++++<br>> 2 files changed, 89 insertions(+), 1 deletion(-)<br>> create mode 100644 test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua<br>><br>> diff --git a/src/luajit.c b/src/luajit.c<br>> index 1ca24301..d335f32b 100644<br>> --- a/src/luajit.c<br>> +++ b/src/luajit.c<br>> @@ -72,6 +72,7 @@ static void print_usage(void)<br>> " -O[opt] Control LuaJIT optimizations.\n"<br>> " -i Enter interactive mode after executing " LUA_QL("script") ".\n"<br>> " -v Show version information.\n"<br>> + " -t[cmd] Execute tool.\n"<br><br>According to usage cmd after -t option is optional. However without cmd<br>luajit prints usage<br><br>that hints about incorrect usage:<br><br><br>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./src/luajit -t<br>usage: ./src/luajit [options]... [script [args]...].<br>Available options are:<br> -e chunk Execute string 'chunk'.<br> -l name Require library 'name'.<br> -b ... Save or list bytecode.<br> -j cmd Perform LuaJIT control command.<br> -O[opt] Control LuaJIT optimizations.<br> -i Enter interactive mode after executing 'script'.<br> -v Show version information.<br> -t[cmd] Execute tool.<br> -E Ignore environment variables.<br> -- Stop handling options.<br> - Execute stdin and stop handling options.</div></div></div></div></blockquote><div>Fixed! Branch is force-pushed, here is the diff:</div><div>=========================================================</div><div><div><div>diff --git a/src/luajit.c b/src/luajit.c</div><div>index d335f32b..c16138f0 100644</div><div>--- a/src/luajit.c</div><div>+++ b/src/luajit.c</div><div>@@ -72,7 +72,7 @@ static void print_usage(void)</div><div> " -O[opt] Control LuaJIT optimizations.\n"</div><div> " -i Enter interactive mode after executing " LUA_QL("script") ".\n"</div><div> " -v Show version information.\n"</div><div>- " -t[cmd] Execute tool.\n"</div><div>+ " -t(cmd) Execute tool.\n"</div><div> " -E Ignore environment variables.\n"</div><div> " -- Stop handling options.\n"</div><div> " - Execute stdin and stop handling options.\n", stderr);</div><div>=========================================================</div></div></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br><snipped><br>> +end<br>> +<br>> +generate_output(TMP_BINFILE, default_payload)<br>> +<br>> +local errcode = os.execute(EXECUTABLE .. ' -tinvalid ' .. TMP_BINFILE)<br>> +test:ok(errcode ~= 0, 'invalid tool flag')<br>> +<br>> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. BAD_PATH)<br>> +test:ok(errcode ~= 0, 'binfile does not exist')<br>> +<br>> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. TMP_BINFILE)<br>> +test:ok(errcode == 0, 'memprof binfile parsing')<br>Nit: probably it is worth adding checks for output.<br>> +<br>> +os.remove(TMP_BINFILE)<br>> +os.exit(test:check() and 0 or 1)<br>> --<br>> 2.39.2 (Apple Git-143)<br>></div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div><div> </div></div></blockquote></BODY></HTML>