Hi, Sergey,

thanks for review!

On 18.02.2025 14:55, Sergey Kaplun via Tarantool-patches wrote:
Hi, Sergey!
Thanks for the patch!
LGTM, with a minor nits below.

On 13.02.25, Sergey Bronnikov wrote:
sysprof has an optional parameter `path`, that set a path to
Typo: s/set/sets/
Fixed.

profiling output file, by default the path is `sysprof.bin`.
Typo: s/profiling/the profiling/
Typo: s/file, by default/file. By default,/

Fixed.



      
`misc.memprof.start()` requires to set a path to profiling output
Typo: s/to set/setting/
Typo: s/profiling/the profiling/
Fixed.

file. The patch fixes this inconsistency by introducing a default
path to memprof profiling output file - `memprof.bin`.
Typo: s/memprof/the memprof/
Fixed.

---
 src/lib_misc.c                                |  5 ++-
 ...misclib-memprof-lapi-default-file.test.lua | 41 +++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua

diff --git a/src/lib_misc.c b/src/lib_misc.c
index 7666d85f..4f5d72fc 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
<snipped>

diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
new file mode 100644
index 00000000..b6233d4a
--- /dev/null
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
@@ -0,0 +1,41 @@
+local tap = require("tap")
Minor: Let's use single quotes here and below.
Side note: This code style part wasn't fixed when the first tests for
memprof was written.
Fixed.

      
+local test = tap.test("misc-memprof-lapi-default-file"):skipcond({
+  ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
+                                               jit.arch ~= "x64",
+  ["Memprof is disabled"] = os.getenv('LUAJIT_DISABLE_MEMPROF'),
+})
+
+test:plan(1)
+
+local tools = require "utils.tools"
Minor: Please use brackets for function calls.
Fixed.

+
+test:test("default-output-file", function(subtest)
+
+  subtest:plan(1)
+
+  local def_output_file = 'memprof.bin'
Minor: s/def/default/
Fixed.

+  os.remove(def_output_file)
+
+  local res, err = misc.memprof.start()
+  -- Should start successfully.
+  assert(res, err)
This assert should be removed since we want to remove the file in case
of an error (for example, we can't write the memprof prologue).
Fixed.

+
+  res, err = misc.memprof.stop()
+  -- Should stop successfully.
+  assert(res, err)
This assert should be removed since we want to remove the file in case
of an error.


Fixed.


+
+  -- Want to cleanup carefully if something went wrong.
+  if not res then
+    os.remove(def_output_file)
+    error(err)
+  end
+
+  local profile_buf = tools.read_file(def_output_file)
+  subtest:ok(profile_buf ~= nil and #profile_buf ~= 0,
+             'default output file is not empty')
+
+  -- We don't need it any more.
+  os.remove(def_output_file)
+end)
+
+test:done(true)
-- 
2.34.1