From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 4223B2A639 for ; Fri, 24 Aug 2018 06:23:31 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RhkxqEi5foAP for ; Fri, 24 Aug 2018 06:23:31 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 01E402A638 for ; Fri, 24 Aug 2018 06:23:30 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] lua: wrong 'tomap' work with nullable fields References: <7d7cd9396f57d25a6bd05a24bf8677e7bb0fdd28.1534934562.git.imeevma@gmail.com> <90fa7a3e-d737-ea85-d4e4-0a0b0ef376d0@tarantool.org> <10fa1a23-736c-9c45-3023-9c69ed7793a1@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5143a8a1-5857-9f43-7135-68999228fcaa@tarantool.org> Date: Fri, 24 Aug 2018 13:23:29 +0300 MIME-Version: 1.0 In-Reply-To: <10fa1a23-736c-9c45-3023-9c69ed7793a1@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Imeev Mergen , tarantool-patches@freelists.org Hi! Thanks for the fixes! > commit 066db38393316c486b9edef0d71e854c3e4fdc78 > Author: Mergen Imeev > Date:   Wed Aug 22 13:33:22 2018 +0300 > >     lua: wrong 'tomap' work with nullable fields > >     Tuple method 'tomap' in some cases worked improperly if tuple >     length less than it should be according to space format. Fixed >     in this patch. > >     Closes #3631 > > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c > index 7ca4299..cd96152 100644 > --- a/src/box/lua/tuple.c > +++ b/src/box/lua/tuple.c > @@ -284,12 +284,11 @@ lbox_tuple_to_map(struct lua_State *L) >          luaL_error(L, "Usage: tuple:tomap()"); >      const struct tuple *tuple = lua_checktuple(L, 1); >      const struct tuple_format *format = tuple_format(tuple); > -    const struct tuple_field *field = &format->fields[0]; >      const char *pos = tuple_data(tuple); >      int field_count = (int)mp_decode_array(&pos); >      int n_named = format->dict->name_count; >      lua_createtable(L, field_count, n_named); > -    for (int i = 0; i < n_named; ++i, ++field) { > +    for (int i = 0; i < MIN(field_count, n_named); ++i) { It is not ok to calculate MIN on each iteration. Either N_named and field_count can not change during iteration. I fixed it in a separate commit on the branch. Please, look and squash. And send the patch as v2 to Vova D. >          /* Access by name. */ >          const char *name = format->dict->names[i]; >          lua_pushstring(L, name); My diff (so far it is required by SOP since recently): diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index cd961526c..5acbecb23 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -288,7 +288,8 @@ lbox_tuple_to_map(struct lua_State *L) int field_count = (int)mp_decode_array(&pos); int n_named = format->dict->name_count; lua_createtable(L, field_count, n_named); - for (int i = 0; i < MIN(field_count, n_named); ++i) { + int named_and_presented = MIN(field_count, n_named); + for (int i = 0; i < named_and_presented; ++i) { /* Access by name. */ const char *name = format->dict->names[i]; lua_pushstring(L, name);