Re: [Tarantool-patches] [PATCH luajit 3/3] sysprof: fix a message with stop without run

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]

Hi, Sergey!

both issues were fixed and force-pushed to the branch.

Sergey

On 07.03.2025 10:21, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the fixes!
> LGTM, with 2 minor comments below.
>
> On 06.03.25, Sergey Bronnikov wrote:
>> When sysprof is not started the function `misc.sysprof.stop()`
> Typo: s/started/started,/
Fixed.
>
>> reports that the profiler is already running:
>>
>> | $ ./src/luajit -e 'print(misc.sysprof.stop())'
>> | nil     profiler is running already     22
>>
>> The patch fixes that:
>>
>> | $ ./src/luajit -e 'print(misc.sysprof.stop())'
>> | nil     profiler is not running         22
>>
>> Follows up tarantool/tarantool#781
>> ---
> <snipped>
>
>> +  if (LJ_UNLIKELY(status == PROFILE_ERRRUN)) {
>> +      lua_pushnil(L);
>> +      lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
>> +      lua_pushinteger(L, EINVAL);
>> +      return 3;
>> +  }
>>     if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
> It looks like more natural now to use `else if` here now.

Fixed, updated version:

--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -336,8 +336,14 @@ LJLIB_CF(misc_sysprof_stop)
    return prof_error(L, PROFILE_ERRUSE, err_details);
  #else
    int status = luaM_sysprof_stop(L);
-  if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
+  if (LJ_UNLIKELY(status == PROFILE_ERRRUN)) {
+    lua_pushnil(L);
+    lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
+    lua_pushinteger(L, EINVAL);
+    return 3;
+  } else if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) {
      return prof_error(L, status, NULL);
+  }

    lua_pushboolean(L, 1);
    return 1;

>
>>       return prof_error(L, status, NULL);
>>   
>> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
>> index 91ea461b..f316c390 100644
>> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
>> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> <snipped>
>
>> -- 
>> 2.43.0
>>

[-- Attachment #2: Type: text/html, Size: 3600 bytes --]