Thank you for the review. Proposed changes look reasonable to me so I’ve squashed your FIXUP commits and also added follow up patch as a separate commit.    >Среда, 6 ноября 2019, 3:44 +03:00 от Alexander Turenko : >  >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 < marianneliash@gmail.com > >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 < alexander.turenko@tarantool.org > >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 < alexander.turenko@tarantool.org > >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 < alexander.turenko@tarantool.org > >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) >      -- Maria Khaydich