[Tarantool-patches] [PATCH] msgpackffi.decode can now be assigned to buf.rpos
Alexander Turenko
alexander.turenko at tarantool.org
Wed Nov 6 03:44:08 MSK 2019
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<char *> 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<char *>, 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<char *>.
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<const char *> (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 at 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 at 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 at 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<const char *>)',
+ data = ffi.cast('const char *', '\x93\x01\x02\x03'),
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode_unchecked(cdata<char *>)',
+ 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 at 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<char *> or cdata<const char *>
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<const char *>, number)',
+ func = s.decode,
+ args = {ffi.cast('const char *', '\x93\x01\x02\x03'), 4},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode(cdata<char *>, number)',
+ func = s.decode,
+ args = {ffi.cast('char *', '\x93\x01\x02\x03'), 4},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode_unchecked(cdata<const char *>)',
+ func = s.decode_unchecked,
+ args = {ffi.cast('const char *', '\x93\x01\x02\x03')},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode_unchecked(cdata<char *>)',
+ 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<char *> arguments are passed in the cases above,
+ -- so only cdata<const char *> 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<const char *>)',
- data = ffi.cast('const char *', '\x93\x01\x02\x03'),
- exp_res = {1, 2, 3},
- exp_rewind = 4,
- },
- {
- 'decode_unchecked(cdata<char *>)',
- 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)
More information about the Tarantool-patches
mailing list