Re: [Tarantool-patches] [tarantool-patches] [PATCH] msgpackffi.decode can now be assigned to buf.rpos

["Maria Khaydich" <maria.khaydich@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2923 bytes --]


Hello, thank you for the review! 
 
> Buffer is never used here. Lets drop it.
 
Done
 
> Now "require('ffi')" in the beginning of that file becomes not used. Please, drop it.
 
And done 
  
>Суббота, 21 декабря 2019, 1:00 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
> 
>
>
>Hi! Thanks for the patch!
>
>Review of the first commit:
>
>============================================================================
>
>> Author: Maria < marianneliash@gmail.com >
>>
>> lua: keeping the pointer type in msgpackffi.decode()
>>
>> Method decode_unchecked returns two values - the one that has
>> been decoded and a pointer to the new position within the buffer
>> given as a parameter. The type of returned pointer used to be
>> cdata<unsigned char *> and it was not possible to assign returned
>> value to buf.rpos due to the following error:
>>
>> > cannot convert 'const unsigned char *' to 'char *'
>>
>> The patch fixes this by making decode_unchecked method return either
>> cdata<char *> or cdata<const char *> depending on the given parameter.
>>
>> Closes #3926
>>
>> diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua
>> index 36ac26b7e..e64228e4d 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')
>
>Buffer is never used here. Lets drop it.
>
>> +local ffi = require('ffi')
>
>============================================================================
>
>Review of the second commit:
>
>============================================================================
>
>> Author: Alexander Turenko < alexander.turenko@tarantool.org >
>>
>> 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/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)
>
>Now "require('ffi')" in the beginning of that file becomes
>not used. Please, drop it.
>
>  
 
 
--
Maria Khaydich
 

[-- Attachment #2: Type: text/html, Size: 4081 bytes --]