Re: [Tarantool-patches] [PATCH luajit 2/3] misc: use prof_error for handling errors

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

Hi, Sergey!
Thanks for the patch!
LGTM, except 1 minor comment and 1 reminder below.

On 25.02.25, Sergey Bronnikov wrote:
> 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);

Discussed offline that this should be done in the previous patch-set.

>  #endif

<snipped>

> +  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))

Minor: Looks like more natural to use `else if` here?
Feel free to ignore.

> +    return prof_error(L, status, NULL);
> +
>    lua_pushboolean(L, 1);
>    return 1;
>  #endif /* !LJ_HASMEMPROF */
> -- 
> 2.43.0
> 

-- 
Best regards,
Sergey Kaplun