[tarantool-patches] Re: [PATCH] msgpackffi.decode can now be assigned to buf.rpos
Georgy Kirichenko
georgy at tarantool.org
Mon Oct 7 11:36:40 MSK 2019
Hi, thanks for the patch. It looks good for me except some non-essential fixes.
I updated your branch, please see and review new version below.
---
src/lua/msgpackffi.lua | 8 +++++---
test/app-tap/msgpackffi.test.lua | 22 +++++++++++++++++++++-
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
index f7ee44291..ac6e8b017 100644
--- a/src/lua/msgpackffi.lua
+++ b/src/lua/msgpackffi.lua
@@ -11,6 +11,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 *
@@ -603,13 +604,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 f2a8f254b..2fd9d4c4b 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(19)
local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47,
@@ -72,7 +91,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)
@@ -85,4 +104,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)
--
2.23.0
On Monday, September 30, 2019 5:44:23 PM MSK Maria wrote:
> 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
>
> Issue:
> https://github.com/tarantool/tarantool/issues/3926
> Branch:
> https://github.com/tarantool/tarantool/tree/eljashm/gh-3926-msgpackffi.decod
> e-assignment-to-buf.rpos ---
> src/lua/msgpackffi.lua | 8 +++++---
> test/app-tap/msgpackffi.test.lua | 20 +++++++++++++++++++-
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
> index f7ee44291..4d2078ffd 100644
> --- a/src/lua/msgpackffi.lua
> +++ b/src/lua/msgpackffi.lua
> @@ -11,6 +11,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 *
> @@ -603,13 +604,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(char_ptr_t, 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 f2a8f254b..9ec1cbbe2 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,21 @@ 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(1)
> + -- 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(p)
> + test:iscdata(new_buf, 'char *', 'cdata type')
> + buf.rpos = new_buf
> + buf:recycle()
> +end
> +
> local function test_other(test, s)
> test:plan(19)
> local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47,
> @@ -72,7 +89,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)
> @@ -85,4 +102,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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20191007/dd879622/attachment.sig>
More information about the Tarantool-patches
mailing list