<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p>see my comments<br>
</p>
<div class="moz-cite-prefix">On 05.03.2025 17:49, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:Z8hkgfxifeW-hc6A@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the patch!
LGTM, except 1 minor comment and 1 reminder below.
On 25.02.25, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">The patch consolidates handling profilers errors into a single
place - in a function `prof_error()` and handles PROFILE_ERRIO,
generated in a function `misc_memprof_start`, in a `prof_error()`.
---
src/lib_misc.c | 49 +++++++++----------------------------------------
1 file changed, 9 insertions(+), 40 deletions(-)
diff --git a/src/lib_misc.c b/src/lib_misc.c
index b4b58509..c4b40996 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -286,14 +286,14 @@ static int prof_error(lua_State *L, int status, const char *err_details)
lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
lua_pushinteger(L, EINVAL);
return 3;
-#if LJ_HASSYSPROF
+#if LJ_HASSYSPROF || LJ_HASMEMPROF
case PROFILE_ERRRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRIO:
- return luaL_fileresult(L, 0, NULL);
+ return luaL_fileresult(L, 0, err_details);
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Discussed offline that this should be done in the previous patch-set.</pre>
</blockquote>
The hunk is gone after rebase.<br>
<blockquote type="cite" cite="mid:Z8hkgfxifeW-hc6A@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre"> #endif
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ if (LJ_UNLIKELY(status == PROFILE_ERRRUN)) {
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
lua_pushinteger(L, EINVAL);
return 3;
- case PROFILE_ERRIO:
- return luaL_fileresult(L, 0, NULL);
-#endif
- default:
- lj_assertL(0, "bad memprof error %d", status);
- return 0;
- }
}
+ if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Minor: Looks like more natural to use `else if` here?
Feel free to ignore.
</pre>
</blockquote>
<p>Fixed:</p>
<p>--- a/src/lib_misc.c<br>
+++ b/src/lib_misc.c<br>
@@ -443,9 +443,9 @@ LJLIB_CF(misc_memprof_stop)<br>
lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));<br>
lua_pushinteger(L, EINVAL);<br>
return 3;<br>
- }<br>
- if (LJ_UNLIKELY(status != PROFILE_SUCCESS))<br>
+ } else if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) {<br>
return prof_error(L, status, NULL);<br>
+ }<br>
<br>
lua_pushboolean(L, 1);<br>
return 1;<br>
</p>
<blockquote type="cite" cite="mid:Z8hkgfxifeW-hc6A@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ return prof_error(L, status, NULL);
+
lua_pushboolean(L, 1);
return 1;
#endif /* !LJ_HASMEMPROF */
--
2.43.0
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>