From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id BEB80130FD93; Thu, 6 Mar 2025 18:18:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BEB80130FD93 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1741274330; bh=52lMsi3AuvRwTIoEQgMwi077/toxJBkQhiyDLuEIkHI=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=oMnz/n1HvLaOWniFEG8CFS45wAWXCtyFGURcG1mOnZln6/jOYTqu1lzhLV89aidzD 3XQh5K87Yx01mn5tnDRC3hLhe0WYOcMHqSV8evdCc0R/nKzpfeZEJMcSUfgYmfo0ar ChrdNx0W2hQG6jIpcWbK0CiCq5Kj00vnNcBI7FQ0= Received: from send196.i.mail.ru (send196.i.mail.ru [95.163.59.35]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 88B70130FD8C for ; Thu, 6 Mar 2025 18:18:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 88B70130FD8C Received: by exim-smtp-8cb569c79-nrshv with esmtpa (envelope-from ) id 1tqCzo-00000000JRC-2Q12; Thu, 06 Mar 2025 18:18:48 +0300 Content-Type: multipart/alternative; boundary="------------9Jxe5wG1utLtcLyRQFBlPdH5" Message-ID: <3eca76ea-3cdc-4595-9b9b-c42a8d3fc082@tarantool.org> Date: Thu, 6 Mar 2025 18:18:48 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun , Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org References: Content-Language: en-US In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9C8AED3E6A44DB6ABDD9CEB5AA6031299DC2A0753C2BEEB50182A05F538085040689E7B1EB396E1B73DE06ABAFEAF670578D3A7EE50C6CB0D812D56C36E52A48986091FA637C0ECF9 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F9D3BE5B596754B8C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6759CC434672EE6371C2A783ECEC0211ADC4224003CC836476D5A39DEEDB180909611E41BBFE2FEB2B75ACB0DC5EBD799CA442E14EC9E97EAA6D91FBC68206F9475E431A1F75C412CC9FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B3733B5EC72352B9FA471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FCF858E60A7739E4253AA81AA40904B5D9CF19DD082D7633A0C84D3B47A649675F3AA81AA40904B5D98AA50765F7900637F3DF60C7B35BBE46D81D268191BDAD3D3666184CF4C3C14F3FC91FA280E0CE3D1A620F70A64A45A98AA50765F79006372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F616AD31D0D18CD5C262FEC7FBD7D1F5BB5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A546D30D883A7954475002B1117B3ED69612FB518E9F6FEE16E99897350C7C491E823CB91A9FED034534781492E4B8EEAD09F854029C6BD0DABDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF9093821501B9A5531D514458EAD7DC1DE34CC24EEA5104302567BD04E5BF630F7DB53F1827808597EDA8A7084F1AF3180664830B880DDE78843F9D6852F39244D1CA2126C7CB93305F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVQiWK+2I7Y2scwfZAFooZd8= X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F258845858909722FB6DD5C88BD48BD36611AD349B36CE67E2BC884087CA5D472DAE1081645D15D82EE4B272BD6E4642A116CA93524AA66B5ACBE6721EF430B9A63E2A504198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/3] sysprof: fix a message with stop without run X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------9Jxe5wG1utLtcLyRQFBlPdH5 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 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 > > >> -- >> 2.43.0 >> --------------9Jxe5wG1utLtcLyRQFBlPdH5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

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


    
--------------9Jxe5wG1utLtcLyRQFBlPdH5--