Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review!
On Tue, Sep 05, 2023 at 02:47:15PM +0300, Sergey Bronnikov wrote:
> Hi, Max
> 
> 
> thanks for the patch! LGTM with a two minor comments.
> 
> 
> It is not related to submitted patches, but anyway - there is a typo:
> 
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -36,7 +36,7 @@ local function default_payload()
>    -- Preallocate table to avoid table array part reallocations.
>    local _ = table_new(20, 0)
> 
> -  -- Want too see 20 objects here.
> +  -- Want to see 20 objects here.
>    for i = 1, 20 do
>      -- Try to avoid crossing with "test" module objects.
>      _[i] = "memprof-str-"..i
I don't really want to change it in scope of this patch set,
so ignoring.
 
> Nit: this change is not relevant. In the branch hunk is different:
> 
> @@ -86,44 +86,17 @@ local function form_source_line(line, source)
>  end
> 
>  local function form_trace_line(traceno, line, source)
> -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> +  return ("TRACE [%d] started at %s:%d"):format(
> +    traceno,
> +    source or SRC_PATH,
> +    line
> +  )
>  end
It is not irrelevant. This patchset required tests to be rewritten,
which resulted in addition of `started at` to this format string
and it went over the 80-character limit.
> 
> <snipped>