<HTML><BODY><div>
<div>Hello, thank you for the review! <br>
</div>
<div>> Buffer is never used here. Lets drop it.</div>
<div> </div>
<div>Done</div>
<div> </div>
<div>> Now "require('ffi')" in the beginning of that file becomes not used. Please, drop it.</div>
<div> </div>
<div>And done </div>
<div> </div>
<blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Суббота, 21 декабря 2019, 1:00 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css">
</style>
<div>
<div id="style_15768792580448825938_BODY"><br>
<br>
Hi! Thanks for the patch!<br>
<br>
Review of the first commit:<br>
<br>
============================================================================<br>
<br>
> Author: Maria <<a href="/compose?To=marianneliash@gmail.com">marianneliash@gmail.com</a>><br>
><br>
> lua: keeping the pointer type in msgpackffi.decode()<br>
><br>
> Method decode_unchecked returns two values - the one that has<br>
> been decoded and a pointer to the new position within the buffer<br>
> given as a parameter. The type of returned pointer used to be<br>
> cdata<unsigned char *> and it was not possible to assign returned<br>
> value to buf.rpos due to the following error:<br>
><br>
> > cannot convert 'const unsigned char *' to 'char *'<br>
><br>
> The patch fixes this by making decode_unchecked method return either<br>
> cdata<char *> or cdata<const char *> depending on the given parameter.<br>
><br>
> Closes #3926<br>
><br>
> diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua<br>
> index 36ac26b7e..e64228e4d 100755<br>
> --- a/test/app-tap/msgpackffi.test.lua<br>
> +++ b/test/app-tap/msgpackffi.test.lua<br>
> @@ -4,6 +4,8 @@ package.path = "lua/?.lua;"..package.path<br>
><br>
> local tap = require('tap')<br>
> local common = require('serializer_test')<br>
> +local buffer = require('buffer')<br>
<br>
Buffer is never used here. Lets drop it.<br>
<br>
> +local ffi = require('ffi')<br>
<br>
============================================================================<br>
<br>
Review of the second commit:<br>
<br>
============================================================================<br>
<br>
> Author: Alexander Turenko <<a href="/compose?To=alexander.turenko@tarantool.org">alexander.turenko@tarantool.org</a>><br>
><br>
> lua: don't modify pointer type in msgpack.decode*<br>
><br>
> msgpackffi.decode_unchecked([const] char *) returns two values: a<br>
> decoded result and a new pointer within passed buffer. After #3926 a<br>
> cdata type of the returned pointer follows a type of passed buffer.<br>
><br>
> This commit modifies behaviour of msgpack module in the same way. The<br>
> following functions now returns cdata<char *> or cdata<const char *><br>
> depending of its argument:<br>
><br>
> * msgpack.decode(cdata<[const] char *>, number)<br>
> * msgpack.decode_unchecked(cdata<[const] char *>)<br>
> * msgpack.decode_array_header(cdata<[const] char *>, number)<br>
> * msgpack.decode_map_header(cdata<[const] char *>, number)<br>
><br>
> Follows up #3926.<br>
><br>
> diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua<br>
> index e64228e4d..be6906e67 100755<br>
> --- a/test/app-tap/msgpackffi.test.lua<br>
> +++ b/test/app-tap/msgpackffi.test.lua<br>
> @@ -117,45 +117,6 @@ local function test_other(test, s)<br>
<br>
Now "require('ffi')" in the beginning of that file becomes<br>
not used. Please, drop it.<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<div> </div>
<div data-signature-widget="container">
<div data-signature-widget="content">
<div>--<br>
Maria Khaydich</div>
</div>
</div>
<div> </div>
</div>
</BODY></HTML>