Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: Maxim Kokryashkin <max.kokryashkin@gmail.com>,
	tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors
Date: Tue, 12 Dec 2023 15:54:46 +0300	[thread overview]
Message-ID: <q2xqvxhgqqpy4d46xee4kdfcnylnpgkrszkkoclttettve5ho3@akojujfyjzv7> (raw)
In-Reply-To: <ZXhJT45dup15Pzhk@root>

Hi, Sergey!
Thanks for the review!
Fixed, branch is force-pushed, tarantool PR is updated.
Iterative diff:
===
diff --git a/Makefile.original b/Makefile.original
index 73995130..4a1e1d9d 100644
--- a/Makefile.original
+++ b/Makefile.original
@@ -97,7 +97,7 @@ FILES_JITLIB= bc.lua bcsave.lua dump.lua p.lua v.lua zone.lua \
 	      dis_x86.lua dis_x64.lua dis_arm.lua dis_arm64.lua \
 	      dis_arm64be.lua dis_ppc.lua dis_mips.lua dis_mipsel.lua \
 	      dis_mips64.lua dis_mips64el.lua vmdef.lua
-FILES_UTILSLIB= avl.lua bufread.lua safe_event_reader.lua symtab.lua
+FILES_UTILSLIB= avl.lua bufread.lua evread.lua symtab.lua
 FILES_MEMPROFLIB= humanize.lua parse.lua process.lua
 FILES_SYSPROFLIB= parse.lua
 FILES_TOOLSLIB= memprof.lua sysprof.lua
diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
index ec958031..8ead83b5 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -42,7 +42,7 @@ local SMOKE_CMD_SET = {
 local MEMPROF_CMD_SET = {
   {
     cmd = MEMPROF_PARSER .. BAD_PATH,
-    like = 'Failed to open',
+    like = 'Failed to open.*fopen, errno: 2',
   },
   {
     cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF,
@@ -61,7 +61,7 @@ local MEMPROF_CMD_SET = {
 local SYSPROF_CMD_SET = {
   {
     cmd = SYSPROF_PARSER .. BAD_PATH,
-    like = 'Failed to open',
+    like = 'Failed to open.*fopen, errno: 2',
   },
   {
     cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF,
diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index 7f728787..695c079a 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -18,7 +18,7 @@ else()
     memprof.lua
     utils/avl.lua
     utils/bufread.lua
-    utils/safe_event_reader.lua
+    utils/evread.lua
     utils/symtab.lua
   )
   list(APPEND LUAJIT_TOOLS_DEPS tools-parse-memprof)
@@ -37,7 +37,7 @@ else()
   install(FILES
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/avl.lua
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/bufread.lua
-      ${CMAKE_CURRENT_SOURCE_DIR}/utils/safe_event_reader.lua
+      ${CMAKE_CURRENT_SOURCE_DIR}/utils/evread.lua
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua
     DESTINATION ${LUAJIT_DATAROOTDIR}/utils
     PERMISSIONS
@@ -67,7 +67,7 @@ else()
     sysprof.lua
     utils/avl.lua
     utils/bufread.lua
-    utils/safe_event_reader.lua
+    utils/evread.lua
     utils/symtab.lua
   )
   list(APPEND LUAJIT_TOOLS_DEPS tools-parse-sysprof)
@@ -84,7 +84,7 @@ else()
   install(FILES
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/avl.lua
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/bufread.lua
-      ${CMAKE_CURRENT_SOURCE_DIR}/utils/safe_event_reader.lua
+      ${CMAKE_CURRENT_SOURCE_DIR}/utils/evread.lua
       ${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua
     DESTINATION ${LUAJIT_DATAROOTDIR}/utils
     PERMISSIONS
diff --git a/tools/memprof.lua b/tools/memprof.lua
index d491b3dc..7d7e8e05 100644
--- a/tools/memprof.lua
+++ b/tools/memprof.lua
@@ -12,7 +12,7 @@

 local memprof = require "memprof.parse"
 local process = require "memprof.process"
-local safe_event_reader = require "utils.safe_event_reader"
+local evread = require "utils.evread"
 local view = require "memprof.humanize"

 local stdout, stderr = io.stdout, io.stderr
@@ -110,7 +110,7 @@ local function dump(inputfile)
   -- XXX: This function exits with a non-zero exit code and
   -- prints an error message if it encounters any failure during
   -- the process of parsing.
-  local events, symbols = safe_event_reader(memprof.parse, inputfile)
+  local events, symbols = evread(memprof.parse, inputfile)

   if not config.leak_only then
     view.profile_info(events, config)
diff --git a/tools/sysprof.lua b/tools/sysprof.lua
index 9c0c23c9..420ca41c 100644
--- a/tools/sysprof.lua
+++ b/tools/sysprof.lua
@@ -1,4 +1,4 @@
-local safe_event_reader = require "utils.safe_event_reader"
+local evread = require "utils.evread"
 local sysprof = require "sysprof.parse"

 local stdout, stderr = io.stdout, io.stderr
@@ -80,7 +80,7 @@ local function dump(inputfile)
   -- XXX: This function exits with a non-zero exit code and
   -- prints an error message if it encounters any failure during
   -- the process of parsing.
-  local events = safe_event_reader(sysprof.parse, inputfile)
+  local events = evread(sysprof.parse, inputfile)

   for stack, count in pairs(events) do
     print(stack, count)
diff --git a/tools/utils/safe_event_reader.lua b/tools/utils/evread.lua
similarity index 82%
rename from tools/utils/safe_event_reader.lua
rename to tools/utils/evread.lua
index 39246a9d..394a4a03 100644
--- a/tools/utils/safe_event_reader.lua
+++ b/tools/utils/evread.lua
@@ -4,12 +4,12 @@ local symtab = require('utils.symtab')
 local function make_error_handler(fmt, inputfile)
   return function(err)
     io.stderr:write(string.format(fmt, inputfile))
-    io.stderr:write(string.format('\nOriginal error: %s', err))
+    io.stderr:write(string.format('\n\t%s\n', err))
     os.exit(1, true)
   end
 end

-local function safe_event_reader(parser, inputfile)
+return function(parser, inputfile)
   local _, reader = xpcall(
     bufread.new,
     make_error_handler('Failed to open %s.', inputfile),
@@ -30,5 +30,3 @@ local function safe_event_reader(parser, inputfile)
   )
   return events, symbols
 end
-
-return safe_event_reader
===

Regards,
Maxim Kokryashkin

  reply	other threads:[~2023-12-12 12:54 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 18:55 [Tarantool-patches] [PATCH luajit v2 0/6] profilers: refactor parsers Maxim Kokryashkin via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 1/6] build: purge sysprof.collapse module Maxim Kokryashkin via Tarantool-patches
2023-12-12 10:18   ` Sergey Kaplun via Tarantool-patches
2023-12-19 12:20   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 2/6] build: fix tool components handling Maxim Kokryashkin via Tarantool-patches
2023-12-12 10:32   ` Sergey Kaplun via Tarantool-patches
2023-12-12 12:53     ` Maxim Kokryashkin via Tarantool-patches
2023-12-12 12:51       ` Sergey Kaplun via Tarantool-patches
2023-12-19 13:56   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 3/6] memprof: refactor `heap_chunk` data structure Maxim Kokryashkin via Tarantool-patches
2023-12-12 10:34   ` Sergey Kaplun via Tarantool-patches
2023-12-19 14:00   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 4/6] memprof: remove unused arguments Maxim Kokryashkin via Tarantool-patches
2023-12-12 10:44   ` Sergey Kaplun via Tarantool-patches
2023-12-19 14:01   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 5/6] memprof: introduce the `--human-readable` option Maxim Kokryashkin via Tarantool-patches
2023-12-12 10:54   ` Sergey Kaplun via Tarantool-patches
2023-12-20 12:24   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 18:55 ` [Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors Maxim Kokryashkin via Tarantool-patches
2023-12-12 11:51   ` Sergey Kaplun via Tarantool-patches
2023-12-12 12:54     ` Maxim Kokryashkin via Tarantool-patches [this message]
2023-12-12 12:54       ` Sergey Kaplun via Tarantool-patches
2023-12-29 12:27   ` Sergey Bronnikov via Tarantool-patches
2024-01-09 13:54     ` Maxim Kokryashkin via Tarantool-patches
2024-01-16  9:48       ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=q2xqvxhgqqpy4d46xee4kdfcnylnpgkrszkkoclttettve5ho3@akojujfyjzv7 \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox