From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 04BF142F4C5 for ; Wed, 6 Nov 2019 03:44:12 +0300 (MSK) Date: Wed, 6 Nov 2019 03:44:08 +0300 From: Alexander Turenko Message-ID: <20191106004408.ajxf53urlqzy37ce@tkn_work_nb> References: <20190912174448.25680-1-maria.khaydich@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20190912174448.25680-1-maria.khaydich@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] msgpackffi.decode can now be assigned to buf.rpos List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Maria Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Note: The patch was changed: now msgpackffi.decode_unchecked() returns a pointer of the same cdata type as passed buffer (the same for its msgpackffi.decode() alias). eljashm/gh-3926-msgpackffi.decode-assignment-to-buf.rpos branch was updated, but the email thread was not. I added the new patch below to make the discussion consistent. The review contains two parts: a question about the API and proposed changes for code and tests. API --- I digged a bit into the history of msgpack and msgpackffi API to understand use cases and decide whether we should always return cdata pointer despite of cdata type of a provided buffer or return cdata<[const] char *> depending on the passed buffer. msgpack module initially accepts only cdata, so no one bother with this question. Then it start to accept const pointers too, while the type of returned pointer remains the same: always cdata. msgpackffi always accepts both types of pointers, but always returns 'unsigned char *'. I guess this was not intentional. I see no reason to don't change the behaviour for both modules to the proposed one: return the same pointer type as passed one. For ones who interested, my sources of info. Here the new API that allows to interact with buffers was introduced: https://github.com/tarantool/tarantool/commit/68c213975a31d0c05b1e5d23e6112417e315f995 https://github.com/tarantool/tarantool/issues/2755 It was needed to easier use ibuf ('buffer' module) here: https://github.com/tarantool/dump/commit/5b1e15e057aa4606fedc929aae51724705600638 Here msgpack module starts to accept cdata (don't sure what the reason was): https://github.com/tarantool/tarantool/commit/453fff2b215e0c2e2d4c5951fae711329eff7b48 Code ---- I'll paste the original (updated as I wrote above) patch and changes I proposed below the email. I also pushed them onto Totktonada/gh-3926-msgpack-decode-retval-ctype branch to ease cherry-picking. Masha, please, look into proposed changes and squash 'FIXUP' commits into your patch (if you're agree with them). Aside of two fixups I made a follow up patch that changes msgpack behaviour is the same way as you changed msgpackffi. Please, look into it too and if you're agree add it on top of your branch too (but as a separate commit). Then, I think somebody else should look into the patchset. I propose to send it to Vlad (because he is the author of 453fff2b). Please, leave me in CC when will send the new version(s) of the patchset. WBR, Alexander Turenko. ---- (The original patch from eljashm/gh-3926-msgpackffi.decode-assignment-to-buf.rpos.) commit 5a8f4504c8f09ff8532dd29da85885bf70abd53b Author: Maria Date: Thu Sep 12 16:55:53 2019 +0300 msgpackffi.decode can now be assigned to buf.rpos Function decode of module msgpackffi was passing value of type const unsigned char * to a C function that accepts arguments of type const char *. Closes #3926 diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua index 5331dc75f..34b22fbb2 100644 --- a/src/lua/msgpackffi.lua +++ b/src/lua/msgpackffi.lua @@ -10,6 +10,7 @@ local uint16_ptr_t = ffi.typeof('uint16_t *') local uint32_ptr_t = ffi.typeof('uint32_t *') local uint64_ptr_t = ffi.typeof('uint64_t *') local const_char_ptr_t = ffi.typeof('const char *') +local char_ptr_t = ffi.typeof('char *') ffi.cdef([[ char * @@ -606,13 +607,14 @@ local function decode_unchecked(str, offset) bufp[0] = buf + offset - 1 local r = decode_r(bufp) return r, bufp[0] - buf + 1 - elseif ffi.istype(const_char_ptr_t, str) then + elseif ffi.istype(const_char_ptr_t, str) or + ffi.istype(char_ptr_t, str) then bufp[0] = str local r = decode_r(bufp) - return r, bufp[0] + return r, ffi.cast(ffi.typeof(str), bufp[0]) else error("msgpackffi.decode_unchecked(str, offset) -> res, new_offset | ".. - "msgpackffi.decode_unchecked(const char *buf) -> res, new_buf") + "msgpackffi.decode_unchecked([const] char *buf) -> res, new_buf") end end diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua index 36ac26b7e..c4276ec43 100755 --- a/test/app-tap/msgpackffi.test.lua +++ b/test/app-tap/msgpackffi.test.lua @@ -4,6 +4,8 @@ package.path = "lua/?.lua;"..package.path local tap = require('tap') local common = require('serializer_test') +local buffer = require('buffer') +local ffi = require('ffi') local function is_map(s) local b = string.byte(string.sub(s, 1, 1)) @@ -35,6 +37,23 @@ local function test_offsets(test, s) test:ok(not pcall(s.decode, dump, offset), "invalid offset") end +local function test_types(test, s) + test:plan(2) + -- gh-3926: decode result cannot be assigned to buffer.rpos + local encoded_data = s.encode(0) + local len = encoded_data:len() + local buf = buffer.ibuf() + buf:reserve(len) + local p = buf:alloc(len) + ffi.copy(p, encoded_data, len) + local _, new_buf = s.decode(ffi.cast(p, 'const char *')) + test:iscdata(new_buf, 'const char *', 'cdata const char * type') + _, new_buf = s.decode(p) + test:iscdata(new_buf, 'char *', 'cdata char * type') + buf.rpos = new_buf + buf:recycle() +end + local function test_other(test, s) test:plan(24) local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47, @@ -117,7 +136,7 @@ end tap.test("msgpackffi", function(test) local serializer = require('msgpackffi') - test:plan(10) + test:plan(11) test:test("unsigned", common.test_unsigned, serializer) test:test("signed", common.test_signed, serializer) test:test("double", common.test_double, serializer) @@ -130,4 +149,5 @@ tap.test("msgpackffi", function(test) --test:test("ucdata", common.test_ucdata, serializer) test:test("offsets", test_offsets, serializer) test:test("other", test_other, serializer) + test:test("types", test_types, serializer) end) ---- commit 6c95d8c4c04e56288dfeacede0bc2468aa292a6d Author: Alexander Turenko Date: Wed Nov 6 01:36:17 2019 +0300 FIXUP: msgpackffi.decode can now be assigned to buf.rpos Added a note that ffi.istype() ignores the const qualifier and changed the code to make it more clear. Saved an argument cdata type to a local variable to make the logic 'return a pointer of the same type as passed one' more clear. No behaviour changes. diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua index 34b22fbb2..bb2747f83 100644 --- a/src/lua/msgpackffi.lua +++ b/src/lua/msgpackffi.lua @@ -9,7 +9,6 @@ local uint8_ptr_t = ffi.typeof('uint8_t *') local uint16_ptr_t = ffi.typeof('uint16_t *') local uint32_ptr_t = ffi.typeof('uint32_t *') local uint64_ptr_t = ffi.typeof('uint64_t *') -local const_char_ptr_t = ffi.typeof('const char *') local char_ptr_t = ffi.typeof('char *') ffi.cdef([[ @@ -603,15 +602,17 @@ end local function decode_unchecked(str, offset) if type(str) == "string" then offset = check_offset(offset, #str) - local buf = ffi.cast(const_char_ptr_t, str) + local buf = ffi.cast(char_ptr_t, str) bufp[0] = buf + offset - 1 local r = decode_r(bufp) return r, bufp[0] - buf + 1 - elseif ffi.istype(const_char_ptr_t, str) or - ffi.istype(char_ptr_t, str) then + elseif ffi.istype(char_ptr_t, str) then + -- Note: ffi.istype() ignores the const qualifier, so both + -- (char *) and (const char *) buffers are valid. bufp[0] = str local r = decode_r(bufp) - return r, ffi.cast(ffi.typeof(str), bufp[0]) + local ctype = ffi.typeof(str) + return r, ffi.cast(ctype, bufp[0]) else error("msgpackffi.decode_unchecked(str, offset) -> res, new_offset | ".. "msgpackffi.decode_unchecked([const] char *buf) -> res, new_buf") ---- commit f1dee13991d43c37d626e91601ab50e34ec2303b Author: Alexander Turenko Date: Wed Nov 6 01:42:28 2019 +0300 FIXUP: msgpackffi.decode can now be assigned to buf.rpos Rewritten the test case. There are several reasons to do so. First, the previous test case implementation uses test:iscdata() which is not sufficient to verify whether the const qualifier is applied to a cdata type, because of using ffi.istype() under hood, which ignores 'const'. Second, the previous test case was invalid, because of wrong order of ffi.cast() arguments. Third, we don't need a real ibuf object, ffi.cast(ctype, 'a string') is enough. This allows to simplify test cases. Fourth, the test case is rewritten in a declarative manner to reduce code duplication. This also allows to expand it w/o many changes for msgpack.decode() function in the following commit. I left only msgpackffi.decode_unchecked() cases. A bit context is needed to describe why. msgpack.decode() accepts two arguments: a buffer and its size (or a string and an offset, but it is out of scope here). msgpack.decode_unchecked() accepts only a buffer (or, again, a string and an offset) and does not verify whether we perform reads out of the buffer bounds. See #2755 for the formal description of the API. msgpackffi module contains only msgpackffi.decode_unchecked(). msgpackffi.decode() is just alias for decode_unchecked: it does not verify a buffer bounds. AFAIU the alias was added to unify testing code for msgpack and msgpackffi modules. Our website has no documentation about msgpackffi. I don't sure whether its API should be considered as public API. However if we'll make the module public, we'll need to implement buffer bound check. And only then state that the API is stable and can be used by users. If we don't consider msgpackffi.decode() as public function for now, it make sense to left it untested when a testing code is not unified between msgpack and msgpackffi modules. However I unified this testing code for both modules in the following commit, so it will be tested anyway: but in the commit where it looks reasonable. diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua index c4276ec43..e64228e4d 100755 --- a/test/app-tap/msgpackffi.test.lua +++ b/test/app-tap/msgpackffi.test.lua @@ -37,23 +37,6 @@ local function test_offsets(test, s) test:ok(not pcall(s.decode, dump, offset), "invalid offset") end -local function test_types(test, s) - test:plan(2) - -- gh-3926: decode result cannot be assigned to buffer.rpos - local encoded_data = s.encode(0) - local len = encoded_data:len() - local buf = buffer.ibuf() - buf:reserve(len) - local p = buf:alloc(len) - ffi.copy(p, encoded_data, len) - local _, new_buf = s.decode(ffi.cast(p, 'const char *')) - test:iscdata(new_buf, 'const char *', 'cdata const char * type') - _, new_buf = s.decode(p) - test:iscdata(new_buf, 'char *', 'cdata char * type') - buf.rpos = new_buf - buf:recycle() -end - local function test_other(test, s) test:plan(24) local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47, @@ -134,6 +117,45 @@ local function test_other(test, s) encode_max_depth = max_depth}) end +-- gh-3926: Ensure that a returned pointer has the same cdata type +-- as passed argument. +local function test_decode_buffer(test, s) + local cases = { + { + 'decode_unchecked(cdata)', + data = ffi.cast('const char *', '\x93\x01\x02\x03'), + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + { + 'decode_unchecked(cdata)', + data = ffi.cast('char *', '\x93\x01\x02\x03'), + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + } + + test:plan(#cases) + + for _, case in ipairs(cases) do + test:test(case[1], function(test) + test:plan(4) + local res, res_buf = s.decode_unchecked(case.data) + test:is_deeply(res, case.exp_res, 'verify result') + local rewind = res_buf - case.data + test:is(rewind, case.exp_rewind, 'verify resulting buffer') + -- test:iscdata() is not sufficient here, because it + -- ignores 'const' qualifier (because of using + -- ffi.istype()). + test:is(type(res_buf), 'cdata', 'verify resulting buffer type') + local data_ctype = tostring(ffi.typeof(case.data)) + local res_buf_ctype = tostring(ffi.typeof(res_buf)) + test:is(res_buf_ctype, data_ctype, 'verify resulting buffer ctype') + end) + end +end + + tap.test("msgpackffi", function(test) local serializer = require('msgpackffi') test:plan(11) @@ -149,5 +171,5 @@ tap.test("msgpackffi", function(test) --test:test("ucdata", common.test_ucdata, serializer) test:test("offsets", test_offsets, serializer) test:test("other", test_other, serializer) - test:test("types", test_types, serializer) + test:test("decode_buffer", test_decode_buffer, serializer) end) ---- commit cc50b71720a46ed947bbdf71d9e3d6c6ade1372a Author: Alexander Turenko Date: Wed Nov 6 02:05:03 2019 +0300 lua: don't modify pointer type in msgpack.decode* msgpackffi.decode_unchecked([const] char *) returns two values: a decoded result and a new pointer within passed buffer. After #3926 a cdata type of the returned pointer follows a type of passed buffer. This commit modifies behaviour of msgpack module in the same way. The following functions now returns cdata or cdata depending of its argument: * msgpack.decode(cdata<[const] char *>, number) * msgpack.decode_unchecked(cdata<[const] char *>) * msgpack.decode_array_header(cdata<[const] char *>, number) * msgpack.decode_map_header(cdata<[const] char *>, number) Follows up #3926. diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c index ef315b692..edbc15b72 100644 --- a/src/lua/msgpack.c +++ b/src/lua/msgpack.c @@ -370,7 +370,8 @@ static int lua_msgpack_decode_cdata(lua_State *L, bool check) { const char *data; - if (luaL_checkconstchar(L, 1, &data) != 0) { + uint32_t cdata_type; + if (luaL_checkconstchar(L, 1, &data, &cdata_type) != 0) { return luaL_error(L, "msgpack.decode: " "a Lua string or 'char *' expected"); } @@ -386,7 +387,7 @@ lua_msgpack_decode_cdata(lua_State *L, bool check) } struct luaL_serializer *cfg = luaL_checkserializer(L); luamp_decode(L, cfg, &data); - *(const char **)luaL_pushcdata(L, CTID_CHAR_PTR) = data; + *(const char **)luaL_pushcdata(L, cdata_type) = data; return 2; } @@ -468,7 +469,8 @@ lua_ibuf_msgpack_decode(lua_State *L) */ static int verify_decode_header_args(lua_State *L, const char *func_name, - const char **data_p, ptrdiff_t *size_p) + const char **data_p, uint32_t *cdata_type_p, + ptrdiff_t *size_p) { /* Verify arguments count. */ if (lua_gettop(L) != 2) @@ -476,7 +478,8 @@ verify_decode_header_args(lua_State *L, const char *func_name, /* Verify ptr type. */ const char *data; - if (luaL_checkconstchar(L, 1, &data) != 0) + uint32_t cdata_type; + if (luaL_checkconstchar(L, 1, &data, &cdata_type) != 0) return luaL_error(L, "%s: 'char *' expected", func_name); /* Verify size type and value. */ @@ -486,6 +489,7 @@ verify_decode_header_args(lua_State *L, const char *func_name, *data_p = data; *size_p = size; + *cdata_type_p = cdata_type; return 0; } @@ -499,8 +503,9 @@ lua_decode_array_header(lua_State *L) { const char *func_name = "msgpack.decode_array_header"; const char *data; + uint32_t cdata_type; ptrdiff_t size; - verify_decode_header_args(L, func_name, &data, &size); + verify_decode_header_args(L, func_name, &data, &cdata_type, &size); if (mp_typeof(*data) != MP_ARRAY) return luaL_error(L, "%s: unexpected msgpack type", func_name); @@ -511,7 +516,7 @@ lua_decode_array_header(lua_State *L) uint32_t len = mp_decode_array(&data); lua_pushinteger(L, len); - *(const char **) luaL_pushcdata(L, CTID_CHAR_PTR) = data; + *(const char **) luaL_pushcdata(L, cdata_type) = data; return 2; } @@ -524,8 +529,9 @@ lua_decode_map_header(lua_State *L) { const char *func_name = "msgpack.decode_map_header"; const char *data; + uint32_t cdata_type; ptrdiff_t size; - verify_decode_header_args(L, func_name, &data, &size); + verify_decode_header_args(L, func_name, &data, &cdata_type, &size); if (mp_typeof(*data) != MP_MAP) return luaL_error(L, "%s: unexpected msgpack type", func_name); @@ -536,7 +542,7 @@ lua_decode_map_header(lua_State *L) uint32_t len = mp_decode_map(&data); lua_pushinteger(L, len); - *(const char **) luaL_pushcdata(L, CTID_CHAR_PTR) = data; + *(const char **) luaL_pushcdata(L, cdata_type) = data; return 2; } diff --git a/src/lua/utils.c b/src/lua/utils.c index 7f7bf4776..8cb9920e0 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -43,8 +43,8 @@ int luaL_array_metatable_ref = LUA_REFNIL; static uint32_t CTID_STRUCT_IBUF; static uint32_t CTID_STRUCT_IBUF_PTR; -uint32_t CTID_CHAR_PTR; -uint32_t CTID_CONST_CHAR_PTR; +static uint32_t CTID_CHAR_PTR; +static uint32_t CTID_CONST_CHAR_PTR; uint32_t CTID_DECIMAL; @@ -1135,7 +1135,8 @@ luaL_checkibuf(struct lua_State *L, int idx) } int -luaL_checkconstchar(struct lua_State *L, int idx, const char **res) +luaL_checkconstchar(struct lua_State *L, int idx, const char **res, + uint32_t *cdata_type_p) { if (lua_type(L, idx) != LUA_TCDATA) return -1; @@ -1144,6 +1145,7 @@ luaL_checkconstchar(struct lua_State *L, int idx, const char **res) if (cdata_type != CTID_CHAR_PTR && cdata_type != CTID_CONST_CHAR_PTR) return -1; *res = cdata != NULL ? *(const char **) cdata : NULL; + *cdata_type_p = cdata_type; return 0; } diff --git a/src/lua/utils.h b/src/lua/utils.h index d9fb0704f..0b3672769 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -71,9 +71,6 @@ struct ibuf; extern struct lua_State *tarantool_L; extern struct ibuf *tarantool_lua_ibuf; -extern uint32_t CTID_CONST_CHAR_PTR; -extern uint32_t CTID_CHAR_PTR; - /** \cond public */ /** @@ -635,7 +632,8 @@ luaL_checkibuf(struct lua_State *L, int idx); * char pointer. */ int -luaL_checkconstchar(struct lua_State *L, int idx, const char **res); +luaL_checkconstchar(struct lua_State *L, int idx, const char **res, + uint32_t *cdata_type_p); /* {{{ Helper functions to interact with a Lua iterator from C */ diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua index 1609f768f..b3e9facc0 100644 --- a/test/app-tap/lua/serializer_test.lua +++ b/test/app-tap/lua/serializer_test.lua @@ -433,6 +433,64 @@ local function test_depth(test, s) s.cfg({encode_deep_as_nil = deep_as_nil, encode_max_depth = max_depth}) end +-- gh-3926: Ensure that a returned pointer has the same cdata type +-- as passed argument. +-- +-- The test case is applicable for msgpack and msgpackffi. +local function test_decode_buffer(test, s) + local cases = { + { + 'decode(cdata, number)', + func = s.decode, + args = {ffi.cast('const char *', '\x93\x01\x02\x03'), 4}, + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + { + 'decode(cdata, number)', + func = s.decode, + args = {ffi.cast('char *', '\x93\x01\x02\x03'), 4}, + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + { + 'decode_unchecked(cdata)', + func = s.decode_unchecked, + args = {ffi.cast('const char *', '\x93\x01\x02\x03')}, + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + { + 'decode_unchecked(cdata)', + func = s.decode_unchecked, + args = {ffi.cast('char *', '\x93\x01\x02\x03')}, + exp_res = {1, 2, 3}, + exp_rewind = 4, + }, + } + + test:plan(#cases) + + for _, case in ipairs(cases) do + test:test(case[1], function(test) + test:plan(4) + local args_len = table.maxn(case.args) + local res, res_buf = case.func(unpack(case.args, 1, args_len)) + test:is_deeply(res, case.exp_res, 'verify result') + local buf = case.args[1] + local rewind = res_buf - buf + test:is(rewind, case.exp_rewind, 'verify resulting buffer') + -- test:iscdata() is not sufficient here, because it + -- ignores 'const' qualifier (because of using + -- ffi.istype()). + test:is(type(res_buf), 'cdata', 'verify resulting buffer type') + local buf_ctype = tostring(ffi.typeof(buf)) + local res_buf_ctype = tostring(ffi.typeof(res_buf)) + test:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype') + end) + end +end + return { test_unsigned = test_unsigned; test_signed = test_signed; @@ -444,4 +502,5 @@ return { test_ucdata = test_ucdata; test_decimal = test_decimal; test_depth = test_depth; + test_decode_buffer = test_decode_buffer; } diff --git a/test/app-tap/msgpack.test.lua b/test/app-tap/msgpack.test.lua index 752f107a8..1df9d2372 100755 --- a/test/app-tap/msgpack.test.lua +++ b/test/app-tap/msgpack.test.lua @@ -136,6 +136,27 @@ local function test_decode_array_map_header(test, s) size = 4, exp_err = end_of_buffer_err, }, + -- gh-3926: Ensure that a returned pointer has the same + -- cdata type as passed argument. + -- + -- cdata arguments are passed in the cases above, + -- so only cdata argument is checked here. + { + 'fixarray (const char *)', + func = s.decode_array_header, + data = ffi.cast('const char *', '\x94'), + size = 1, + exp_len = 4, + exp_rewind = 1, + }, + { + 'fixmap (const char *)', + func = s.decode_map_header, + data = ffi.cast('const char *', '\x84'), + size = 1, + exp_len = 4, + exp_rewind = 1, + }, } local bad_api_cases = { @@ -206,9 +227,14 @@ local function test_decode_array_map_header(test, s) else local len, new_buf = case.func(case.data, case.size) local rewind = new_buf - case.data + + -- gh-3926: Verify cdata type of a returned buffer. + local data_ctype = tostring(ffi.typeof(case.data)) + local new_buf_ctype = tostring(ffi.typeof(new_buf)) + local description = ('good; %s'):format(case[1]) - test:is_deeply({len, rewind}, {case.exp_len, case.exp_rewind}, - description) + test:is_deeply({len, rewind, new_buf_ctype}, {case.exp_len, + case.exp_rewind, data_ctype}, description) end end @@ -231,7 +257,7 @@ end tap.test("msgpack", function(test) local serializer = require('msgpack') - test:plan(12) + test:plan(13) test:test("unsigned", common.test_unsigned, serializer) test:test("signed", common.test_signed, serializer) test:test("double", common.test_double, serializer) @@ -244,4 +270,5 @@ tap.test("msgpack", function(test) test:test("offsets", test_offsets, serializer) test:test("misc", test_misc, serializer) test:test("decode_array_map", test_decode_array_map_header, serializer) + test:test("decode_buffer", common.test_decode_buffer, serializer) end) diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua index e64228e4d..be6906e67 100755 --- a/test/app-tap/msgpackffi.test.lua +++ b/test/app-tap/msgpackffi.test.lua @@ -117,45 +117,6 @@ local function test_other(test, s) encode_max_depth = max_depth}) end --- gh-3926: Ensure that a returned pointer has the same cdata type --- as passed argument. -local function test_decode_buffer(test, s) - local cases = { - { - 'decode_unchecked(cdata)', - data = ffi.cast('const char *', '\x93\x01\x02\x03'), - exp_res = {1, 2, 3}, - exp_rewind = 4, - }, - { - 'decode_unchecked(cdata)', - data = ffi.cast('char *', '\x93\x01\x02\x03'), - exp_res = {1, 2, 3}, - exp_rewind = 4, - }, - } - - test:plan(#cases) - - for _, case in ipairs(cases) do - test:test(case[1], function(test) - test:plan(4) - local res, res_buf = s.decode_unchecked(case.data) - test:is_deeply(res, case.exp_res, 'verify result') - local rewind = res_buf - case.data - test:is(rewind, case.exp_rewind, 'verify resulting buffer') - -- test:iscdata() is not sufficient here, because it - -- ignores 'const' qualifier (because of using - -- ffi.istype()). - test:is(type(res_buf), 'cdata', 'verify resulting buffer type') - local data_ctype = tostring(ffi.typeof(case.data)) - local res_buf_ctype = tostring(ffi.typeof(res_buf)) - test:is(res_buf_ctype, data_ctype, 'verify resulting buffer ctype') - end) - end -end - - tap.test("msgpackffi", function(test) local serializer = require('msgpackffi') test:plan(11) @@ -171,5 +132,5 @@ tap.test("msgpackffi", function(test) --test:test("ucdata", common.test_ucdata, serializer) test:test("offsets", test_offsets, serializer) test:test("other", test_other, serializer) - test:test("decode_buffer", test_decode_buffer, serializer) + test:test("decode_buffer", common.test_decode_buffer, serializer) end)