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: 2637 bytes --]

Hi, Sergey,

the patch series was rebased to the master branch (tarantool/master)

without conflicts.

Sergey

On 05.03.2025 17:52, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, with a few nits regarding the commit message and 1 ignorable
> comment to the code style.
>
> On 25.02.25, Sergey Bronnikov wrote:
>> The function `misc.sysprof.stop()` reports that profiler is
> Typo: s/profiler/the profiler/
>
>> already running:
>>
>> | $ ./src/luajit -e 'print(misc.sysprof.stop())'
>> | nil     profiler is running already     22
>>
>> both in `sysprof_error()` and fixes aforementioned problem.
> Don't get this sentence.

Missed this comment.

Now commit message is updated, new version is below:

Author: Sergey Bronnikov <sergeyb@tarantool.org>
Date:   Tue Feb 4 11:48:05 2025 +0300

     sysprof: fix a message with stop without run

     When sysprof is not started the function `misc.sysprof.stop()`
     reports that 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

>
> Typo: s/aforementioned/the aforementioned/
>
>> Follows up tarantool/tarantool#781
>> ---
>>   src/lib_misc.c                                              | 6 ++++++
>>   .../tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 5 +++--
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/lib_misc.c b/src/lib_misc.c
>> index c4b40996..74888d20 100644
>> --- a/src/lib_misc.c
>> +++ b/src/lib_misc.c
>> @@ -335,6 +335,12 @@ 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_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.
>
>>       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 ebd80cf6..770b5736 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: 4148 bytes --]