Re: [Tarantool-patches] [PATCH] lua: assert on lua_gettop() negative return value

["Ilya Kosarev" <i.kosarev@tarantool.org>]

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


Hi!
 
Thanks for the review.
 
Right, sent v2 considering your comments.
  
>Понедельник, 20 июля 2020, 13:50 +03:00 от Nikita Pettik <korablev@tarantool.org>:
> 
>On 17 Jul 20:25, Ilya Kosarev wrote:
>> In case lua_gettop() called from encode_lua_call() returns negative
>> value, we will segfault in iproto_reply_error() with empty diag, as far
>> as it is unexpected error path not covered with diagnostics. Thus
>> corresponding sane check with assert is introduced.
>>
>> Closes #4649
>> ---
>> Branch:  https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4649-sane-check-on-lua_gettop
>> Issue:  https://github.com/tarantool/tarantool/issues/4649
>>
>> @ChangeLog:
>> * Raise assert on lua_gettop() negative return value (gh-4649).
>
>I guess changelog is likely to be redundant in this case
>(patch doesnt introduce user-visible changes).
> 
>> src/box/lua/call.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
>> index ca871e077..75634de5b 100644
>> --- a/src/box/lua/call.c
>> +++ b/src/box/lua/call.c
>> @@ -361,6 +361,7 @@ encode_lua_call(lua_State *L)
>>
>> struct luaL_serializer *cfg = luaL_msgpack_default;
>> int size = lua_gettop(port->L);
>> + assert(size >= 0);
>
>I don't understand much in Lua internals, so it would be nice
>to see rationale for this check. Even if you don't have
>complete insight concerning it, any thoughts which bring you
>to this check will be helpful.
>
>> for (int i = 1; i <= size; ++i)
>> luamp_encode(port->L, cfg, &stream, i);
>> port->size = size;
>> --
>> 2.17.1
>> 
 
 
--
Ilya Kosarev
 

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