From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] [PATCH v1 1/1] netbox: follow-ups References: <95dd3d5749e69c319679dd7e6ff37c2dcc353aaa.1561200656.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: <9a7001b9-d23f-434c-fb2e-a4504f4b6414@tarantool.org> Date: Sat, 22 Jun 2019 16:21:58 +0200 MIME-Version: 1.0 In-Reply-To: <95dd3d5749e69c319679dd7e6ff37c2dcc353aaa.1561200656.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: imeevma@tarantool.org Cc: vdavydov.dev@gmail.com, tarantool-patches@freelists.org List-ID: Hi! Thanks for your fixes. I appreciate that you decided to fix the patch after push. See 2 comments below. >> Thanks for the patch! >> >>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c >>> @@ -618,6 +618,13 @@ static int >>> netbox_decode_select(struct lua_State *L) >>> { >>> uint32_t ctypeid; >>> + int top = lua_gettop(L); >>> + assert(top == 1 || top == 2); >>> + struct tuple_format *format; >>> + if (top == 2 && lua_type(L, 2) == LUA_TCDATA) >>> + format = lbox_check_tuple_format(L, 2); >> >> How is it possible, that we do not have a format here? >> > > it is possible since decode_call_16 calls select without format. 1. Oh, I forgot that call16 uses select decoder. > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > index 251ad40..f7e1688 100644 > --- a/src/box/lua/net_box.lua > +++ b/src/box/lua/net_box.lua > @@ -64,12 +64,12 @@ local function decode_data(raw_data) > local response, raw_end = decode(raw_data) > return response[IPROTO_DATA_KEY], raw_end > end > -local function decode_tuple(raw_data) > - local response, raw_end = internal.decode_select(raw_data) > +local function decode_tuple(raw_data, raw_data_end, format) > + local response, raw_end = internal.decode_select(raw_data, format) > return response[1], raw_end > end > -local function decode_get(raw_data) > - local body, raw_end = internal.decode_select(raw_data) > +local function decode_get(raw_data, raw_data_end, format) > + local body, raw_end = internal.decode_select(raw_data, format) > if body[2] then > return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE > end > @@ -83,6 +83,12 @@ local function decode_push(raw_data) > local response, raw_end = decode(raw_data) > return response[IPROTO_DATA_KEY][1], raw_end > end > +local function decode_call_16(raw_data) > + return internal.decode_select(raw_data) > +end 2. Why do you need that wrapper now? You could keep direct reference to internal.decode_select in method_decoder table. Couldn't you? > +local function decode_select(raw_data, raw_data_end, format) > + return internal.decode_select(raw_data, format) > +end > See my proposals below and on the branch in a separate commit: =================================================================== diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc index 3340070b0..a0a72aa28 100644 --- a/src/box/lua/misc.cc +++ b/src/box/lua/misc.cc @@ -201,6 +201,7 @@ lbox_tuple_format_new(struct lua_State *L) if (fields == NULL) { diag_set(OutOfMemory, size, "region_alloc", "fields[i].name"); + region_truncate(region, region_svp); return luaT_error(L); } memcpy(fields[i].name, name, len); diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c index ad7bc6adf..a30105988 100644 --- a/src/box/lua/net_box.c +++ b/src/box/lua/net_box.c @@ -619,7 +619,7 @@ netbox_decode_select(struct lua_State *L) { uint32_t ctypeid; int top = lua_gettop(L); - assert(top == 1 || top == 2); + assert(top == 1 || top == 2 || top == 3 && lua_isnil(L, 3)); struct tuple_format *format; if (top == 2 && lua_type(L, 2) == LUA_TCDATA) format = lbox_check_tuple_format(L, 2); diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index f7e1688bb..c0e85e1c9 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -83,9 +83,6 @@ local function decode_push(raw_data) local response, raw_end = decode(raw_data) return response[IPROTO_DATA_KEY][1], raw_end end -local function decode_call_16(raw_data) - return internal.decode_select(raw_data) -end local function decode_select(raw_data, raw_data_end, format) return internal.decode_select(raw_data, format) end @@ -116,7 +113,7 @@ local method_encoder = { local method_decoder = { ping = decode_nil, - call_16 = decode_call_16, + call_16 = internal.decode_select, call_17 = decode_data, eval = decode_data, insert = decode_tuple,