Re: [Tarantool-patches] [PATCH luajit 6/7] misc: specific message for disabled profilers

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

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

Hi,

On 19.02.2025 18:41, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the fixes!
>
> On 19.02.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for review!
>>
>> On 19.02.2025 11:06, Sergey Kaplun via Tarantool-patches wrote:
> <snipped>
>
>>> It's more LuaJIT-way to use something like the following:
>>> | if (!LJ_HASSYSPROF) {
>>> |   /* ... */
>>> | }
>>>
>>> This helps to avoid strange early return.
>> Fixed.
>>> Also, please be avaired to declare all variables (stataus, opt,
>>> err_details) in the beginning of the block.
>>    It is a requirement for the code that strictly follows c89, when you
>> must declare all of your variables at the beginning of a scope block.
>>
>> AFAIK, we have no such requirement, and also there is no option
>> "-std=c89" in CI and no any mentions in the contribution guide.
> This is not mentioned in Tarantool's contribution guide, since it is
> LuaJIT-specific. In the perfect world, it should be checked by the flags
> mentioned in <src/Makefile.original> (see the second CWARN).
> Unfortunately, there are some bugs that should be fixed first.
>
> <snipped>

Fixed:


diff --git a/src/lib_misc.c b/src/lib_misc.c
index aaf08718..7b934e52 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -312,15 +312,14 @@ static int sysprof_error(lua_State *L, int status, 
const char *err_details)
  LJLIB_CF(misc_sysprof_start)
  {
    const char *err_details = NULL;
+  int status = PROFILE_SUCCESS;
+  struct luam_Sysprof_Options opt = {};
+
    if (!LJ_HASSYSPROF) {
      err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
      return sysprof_error(L, PROFILE_ERRUSE, err_details);
    }

-  int status = PROFILE_SUCCESS;
-
-  struct luam_Sysprof_Options opt = {};
-
    status = parse_sysprof_opts(L, &opt, &err_details);
    if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
      return sysprof_error(L, status, err_details);
@@ -337,11 +336,11 @@ LJLIB_CF(misc_sysprof_start)
  /* local res, err, errno = profile.sysprof_stop() */
  LJLIB_CF(misc_sysprof_stop)
  {
+  int status = luaM_sysprof_stop(L);
    if (!LJ_HASSYSPROF) {
      const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
      return sysprof_error(L, PROFILE_ERRUSE, err_details);
    }
-  int status = luaM_sysprof_stop(L);
    if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
      return sysprof_error(L, status, NULL);

@@ -352,15 +351,15 @@ LJLIB_CF(misc_sysprof_stop)
  /* local counters, err, errno = sysprof.report() */
  LJLIB_CF(misc_sysprof_report)
  {
+  struct luam_Sysprof_Counters counters = {};
+  GCtab *data_tab = NULL;
+  GCtab *count_tab = NULL;
+  int status = luaM_sysprof_report(&counters);
    if (!LJ_HASSYSPROF) {
      const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
      return sysprof_error(L, PROFILE_ERRUSE, err_details);
    }
-  struct luam_Sysprof_Counters counters = {};
-  GCtab *data_tab = NULL;
-  GCtab *count_tab = NULL;

-  int status = luaM_sysprof_report(&counters);
    if (status != PROFILE_SUCCESS)
      return sysprof_error(L, status, NULL);

@@ -395,14 +394,14 @@ LJLIB_CF(misc_sysprof_report)
  /* local started, err, errno = misc.memprof.start(fname) */
  LJLIB_CF(misc_memprof_start)
  {
-  if (!LJ_HASMEMPROF) {
-    const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
-    return sysprof_error(L, PROFILE_ERRUSE, err_details);
-  }
    struct lj_memprof_options opt = {0};
    const char *fname = strdata(lj_lib_checkstr(L, 1));
    struct profile_ctx *ctx;
    int memprof_status;
+  if (!LJ_HASMEMPROF) {
+    const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+    return sysprof_error(L, PROFILE_ERRUSE, err_details);
+  }

    /*
    ** FIXME: more elegant solution with ctx.
@@ -453,11 +452,11 @@ LJLIB_CF(misc_memprof_start)
  /* local stopped, err, errno = misc.memprof.stop() */
  LJLIB_CF(misc_memprof_stop)
  {
+  int status = lj_memprof_stop(L);
    if (!LJ_HASMEMPROF) {
      const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
      return sysprof_error(L, PROFILE_ERRUSE, err_details);
    }
-  int status = lj_memprof_stop(L);
    if (status != PROFILE_SUCCESS) {
      switch (status) {
      case PROFILE_ERRUSE:

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