From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option Date: Sat, 14 Sep 2019 00:32:34 +0200 [thread overview] Message-ID: <75b097fc-9cef-345a-07ad-e328a55df819@tarantool.org> (raw) In-Reply-To: <20190912232405.qrig7tt5dj7uvf3j@tkn_work_nb> Thanks for the review! On 13/09/2019 01:24, Alexander Turenko wrote: > LGTM. > > One small comment about the test case, see below. > > WBR, Alexander Turenko. > >> + -- >> + -- gh-4434: msgpackffi does not care about msgpack serializer >> + -- configuration, but it should. >> + -- >> + local function check_depth(depth_to_try) >> + local t = nil >> + for i = 1, depth_to_try do t = {t} end >> + t = s.decode_unchecked(s.encode(t)) >> + local level = 0 >> + while t ~= nil do level = level + 1 t = t[1] end >> + return level >> + end >> + local msgpack = require('msgpack') >> + local max_depth = msgpack.cfg.encode_max_depth >> + local result_depth = check_depth(max_depth + 5) >> + test:is(result_depth, max_depth, >> + "msgpackffi uses msgpack.cfg.encode_max_depth") >> + >> + msgpack.cfg({encode_max_depth = max_depth + 5}) >> + result_depth = check_depth(max_depth + 5) >> + test:is(result_depth, max_depth + 5, "and uses it dynamically") >> + >> + msgpack.cfg({encode_max_depth = max_depth}) > > Just in case: we recently close an [issue][1] when json module handles > max depth correctly for maps, but non-correctly for arrays. I think it > worth to ensure that those cases works good both for msgpackffi too. > > [1]: https://github.com/tarantool/tarantool/issues/4366 > Ok: diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua index e26247625..a1028e0ba 100755 --- a/test/app-tap/msgpackffi.test.lua +++ b/test/app-tap/msgpackffi.test.lua @@ -36,7 +36,7 @@ local function test_offsets(test, s) end local function test_other(test, s) - test:plan(21) + test:plan(23) local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47, 0x6f, 0xff, 0xff, 0xac, 0x77, 0x6b, 0x61, 0x71, 0x66, 0x7a, 0x73, 0x7a, 0x75, 0x71, 0x71, 0x78) @@ -91,6 +91,20 @@ local function test_other(test, s) result_depth = check_depth(max_depth + 5) test:is(result_depth, max_depth + 5, "and uses it dynamically") + -- Recursive tables are handled correctly. + local level = 0 + local t = {} + t[1] = t + t = s.decode(s.encode(t)) + while t ~= nil do level = level + 1 t = t[1] end + test:is(level, max_depth + 5, "recursive array") + t = {} + t.key = t + t = s.decode(s.encode(t)) + level = 0 + while t ~= nil do level = level + 1 t = t.key end + test:is(level, max_depth + 5, "recursive map") + msgpack.cfg({encode_max_depth = max_depth}) end
next prev parent reply other threads:[~2019-09-13 22:28 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-10 20:24 [tarantool-patches] [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 1/4] app: serializers update now is reflected in Lua Vladislav Shpilevoy 2019-09-12 23:22 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option Vladislav Shpilevoy 2019-09-12 23:24 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy [this message] 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple Vladislav Shpilevoy 2019-09-12 23:27 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 4/4] app: allow to raise an error on too nested tables Vladislav Shpilevoy 2019-09-12 23:32 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-10 20:25 ` [tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy 2019-09-12 23:44 ` Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy
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=75b097fc-9cef-345a-07ad-e328a55df819@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option' \ /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