Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: vdavydov.dev@gmail.com
Cc: tarantool-patches@freelists.org, kostja@tarantool.org,
	Serge Petrenko <sergepetrenko@tarantool.org>
Subject: [PATCH] xrow: improve corrupted header logging on an error
Date: Fri,  5 Apr 2019 19:09:17 +0300	[thread overview]
Message-ID: <20190405160918.17305-1-sergepetrenko@tarantool.org> (raw)

Improve row printing to log. Since say only has 16k buffer, there is no
point in printing the whole packet, which can have arbitrary length, in one
go.
So, print the header row by row, 16 bytes in a row, and format output to
match `xxd` output:
```
[001] 2019-04-05 18:22:46.679 [11859] iproto V> Got a corrupted row:
[001] 2019-04-05 18:22:46.679 [11859] iproto V> 00000000: A3 02 D6 5A E4 D9 E7 68 A1 53 8D 53 60 5F 20 3F
[001] 2019-04-05 18:22:46.679 [11859] iproto V> 00000010: D8 E2 D6 E2 A3 02 D6 5A E4 D9 E7 68 A1 53 8D 53
```
Now we can get rid of malloc, and use a preallocated tt_static_buf
instead.
Also, replace a big macro with a small macro and a helper function.

Followup to f645119f7732b67ed50b48b1b9d7c8b683953931
---
https://github.com/tarantool/tarantool/tree/sp/print-bad-row-followup

 src/box/xrow.c            | 55 +++++++++++++++++++--------------------
 test/box/net.box.result   | 30 +++++++++++++++++----
 test/box/net.box.test.lua | 13 ++++++---
 3 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/src/box/xrow.c b/src/box/xrow.c
index 6aebe5c50..aaed84e38 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -89,37 +89,36 @@ mp_decode_vclock(const char **data, struct vclock *vclock)
 }
 
 /**
- * Set diagnostics with an error and log the corrupted row
- * which caused the error.
- * Optionally, if log_level is 'verbose' or greater,
+ * If log_level is 'verbose' or greater,
  * dump the corrupted row contents in hex to the log.
  *
- * @param what - exception to set.
- * @param desc_str - error description string.
+ * The format is similar to the xxd utility.
  */
-#define xrow_on_decode_err(start, end, what, desc_str) do {				\
-	diag_set(ClientError, what, desc_str);					\
-											\
-	if (!say_log_level_is_enabled(S_VERBOSE))					\
-		break;									\
-											\
-	size_t len = 3 * ((const char *)(end) - (const char *)(start) + 1);		\
-	char *buf = (char *)malloc(len);						\
-	if (!buf) {									\
-		say_verbose("Got a corrupted row during decoding. "			\
-			    "Not enough memory to dump row contents.");			\
-		break;									\
-	}										\
-											\
-	char *pos = buf;								\
-	char *buf_end = buf + len;							\
-	for (const char *cur = start; cur < end; ++cur) {				\
-		pos += snprintf(pos, buf_end - pos, "%02X ", (unsigned char)*cur);	\
-	}										\
-	say_verbose("Corrupted row is: %s", buf);					\
-											\
-	free(buf);									\
-} while (0)
+void dump_row_hex(const char *start, const char *end) {
+	if (!say_log_level_is_enabled(S_VERBOSE))
+		return;
+
+	char *buf = tt_static_buf();
+	const char *buf_end = buf + TT_STATIC_BUF_LEN;
+
+	say_verbose("Got a corrupted row:");
+	for (const char *cur = start; cur < end;) {
+		char *pos = buf;
+		pos += snprintf(pos, buf_end - pos, "%08lX: ", cur - start);
+		for (size_t i = 0; i < 16; ++i) {
+			pos += snprintf(pos, buf_end - pos, "%02X ", (unsigned char)*cur++);
+			if (cur >= end || pos == buf_end)
+				break;
+		}
+		*pos = '\0';
+		say_verbose("%s", buf);
+	}
+}
+
+#define xrow_on_decode_err(start, end, what, desc_str) do {\
+	diag_set(ClientError, what, desc_str);\
+	dump_row_hex(start, end);\
+} while (0);
 
 int
 xrow_header_decode(struct xrow_header *header, const char **pos,
diff --git a/test/box/net.box.result b/test/box/net.box.result
index a9f1b8f3f..f71699818 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3541,21 +3541,41 @@ sock = socket.tcp_connect(LISTEN.host, LISTEN.service)
 -- we need to have a packet with correctly encoded length,
 -- so that it bypasses iproto length check, but cannot be
 -- decoded in xrow_header_decode
--- 0x14 = 20, sha1 digest is 20 bytes long
-data = string.fromhex('14'..require('digest').sha1_hex('asdf'))
+-- 0x3C = 60, sha1 digest is 20 bytes long
+data = string.fromhex('3C'..string.rep(require('digest').sha1_hex('bcde'), 3))
 ---
 ...
 sock:write(data)
 ---
-- 21
+- 61
 ...
 sock:close()
 ---
 - true
 ...
-test_run:grep_log('default', 'Corrupted row is: .*')
+test_run:grep_log('default', 'Got a corrupted row.*')
+---
+- 'Got a corrupted row:'
+...
+test_run:grep_log('default', '00000000:.*')
+---
+- '00000000: A3 02 D6 5A E4 D9 E7 68 A1 53 8D 53 60 5F 20 3F '
+...
+test_run:grep_log('default', '00000010:.*')
+---
+- '00000010: D8 E2 D6 E2 A3 02 D6 5A E4 D9 E7 68 A1 53 8D 53 '
+...
+test_run:grep_log('default', '00000020:.*')
+---
+- '00000020: 60 5F 20 3F D8 E2 D6 E2 A3 02 D6 5A E4 D9 E7 68 '
+...
+test_run:grep_log('default', '00000030:.*')
 ---
-- 'Corrupted row is: 3D A5 41 55 99 18 A8 08 C2 40 2B BA 50 12 F6 C6 0B 27 66 1C '
+- '00000030: A1 53 8D 53 60 5F 20 3F D8 E2 D6 E2 '
+...
+test_run:grep_log('default', '00000040:.*')
+---
+- null
 ...
 box.cfg{log_level=log_level}
 ---
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index e2f8f90ba..beb33c24f 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1445,9 +1445,16 @@ sock = socket.tcp_connect(LISTEN.host, LISTEN.service)
 -- we need to have a packet with correctly encoded length,
 -- so that it bypasses iproto length check, but cannot be
 -- decoded in xrow_header_decode
--- 0x14 = 20, sha1 digest is 20 bytes long
-data = string.fromhex('14'..require('digest').sha1_hex('asdf'))
+-- 0x3C = 60, sha1 digest is 20 bytes long
+data = string.fromhex('3C'..string.rep(require('digest').sha1_hex('bcde'), 3))
 sock:write(data)
 sock:close()
-test_run:grep_log('default', 'Corrupted row is: .*')
+
+test_run:grep_log('default', 'Got a corrupted row.*')
+test_run:grep_log('default', '00000000:.*')
+test_run:grep_log('default', '00000010:.*')
+test_run:grep_log('default', '00000020:.*')
+test_run:grep_log('default', '00000030:.*')
+test_run:grep_log('default', '00000040:.*')
+
 box.cfg{log_level=log_level}
-- 
2.20.1 (Apple Git-117)

             reply	other threads:[~2019-04-05 16:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-05 16:09 Serge Petrenko [this message]
2019-04-07 11:45 ` Vladimir Davydov

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=20190405160918.17305-1-sergepetrenko@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH] xrow: improve corrupted header logging on an error' \
    /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