[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