<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Max, thanks for the <s>cake</s> patches! LGTM</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 7/14/23 15:41, Maxim Kokryashkin
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1689338470.607537260@f763.i.mail.ru">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<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"
moz-do-not-send="true">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" moz-do-not-send="true"
class="moz-txt-link-freetext">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" moz-do-not-send="true"
class="moz-txt-link-freetext">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>
</blockquote>
</body>
</html>