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 C9A002743C for ; Thu, 26 Jul 2018 06:07:40 -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 jzLrMApovZ0l for ; Thu, 26 Jul 2018 06:07:40 -0400 (EDT) Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 E90892742F for ; Thu, 26 Jul 2018 06:07:39 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3] json: add options to json.encode() References: <693ca18f728ee462e28861841004ff77bf62bfb5.1531010828.git.roman.habibov1@yandex.ru> <8327181531851586@myt4-ec1fcebe7be6.qloud-c.yandex.net> <12c20a86-8a40-4fe6-8dab-7b7b9494a142@tarantool.org> <39430371532385504@iva8-37fc2ad204cd.qloud-c.yandex.net> <3367531532598057@sas1-4b7566131ec9.qloud-c.yandex.net> From: Vladislav Shpilevoy Message-ID: <0e2c47c2-0276-a1da-c54b-726b64c17aae@tarantool.org> Date: Thu, 26 Jul 2018 13:07:37 +0300 MIME-Version: 1.0 In-Reply-To: <3367531532598057@sas1-4b7566131ec9.qloud-c.yandex.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: roman.habibov1@yandex.ru, "tarantool-patches@freelists.org" Hi! 1. Build error. [ 66%] Building C object src/CMakeFiles/server.dir/__/third_party/lua-cjson/lua_cjson.c.o /Users/v.shpilevoy/Work/Repositories/tarantool/src/lua/utils.c:250:13: error: use of undeclared identifier 'l' lua_pop(l, 1); ^ 1 error generated. 2. The patch you sent below differs from the one on the branch. For example, first diff lines on the branch: diff --git a/src/lua/utils.c b/src/lua/utils.c index 2f0f4dcf8..012758b7f 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -186,7 +186,6 @@ luaL_setcdatagc(struct lua_State *L, int idx) lua_pop(L, 1); } - #define OPTION(type, name, defvalue) { #name, \ offsetof(struct luaL_serializer, name), type, defvalue} /** @@ -214,6 +213,44 @@ static struct { But below I see another diff. On 26/07/2018 12:40, roman.habibov1@yandex.ru wrote: > Sorry. https://github.com/tarantool/tarantool/commit/768b875e498a17b3d5e404af79c1119b10a966e3 > > diff --git a/src/lua/utils.c b/src/lua/utils.c > index 0626bf76b..012758b7f 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -186,19 +186,35 @@ luaL_setcdatagc(struct lua_State *L, int idx) > lua_pop(L, 1); > } > > +#define OPTION(type, name, defvalue) { #name, \ > + offsetof(struct luaL_serializer, name), type, defvalue} > /** > - * @brief serializer.cfg{} Lua binding for serializers. > - * serializer.cfg is a table that contains current configuration values from > - * luaL_serializer structure. serializer.cfg has overriden __call() method > - * to change configuration keys in internal userdata (like box.cfg{}). > - * Please note that direct change in serializer.cfg.key will not affect > - * internal state of userdata. > - * @param L lua stack > - * @return 0 > + * Configuration options for serializers > + * @sa struct luaL_serializer > */ > - > -/*Serialize one option. Returns ponter to the value of option.*/ > -int* parse_option(lua_State *L, int i, struct luaL_serializer* cfg) { > +static struct { > + const char *name; > + size_t offset; /* offset in structure */ > + int type; > + int defvalue; > +} OPTIONS[] = { > + OPTION(LUA_TBOOLEAN, encode_sparse_convert, 1), > + OPTION(LUA_TNUMBER, encode_sparse_ratio, 2), > + OPTION(LUA_TNUMBER, encode_sparse_safe, 10), > + OPTION(LUA_TNUMBER, encode_max_depth, 32), > + OPTION(LUA_TBOOLEAN, encode_invalid_numbers, 1), > + OPTION(LUA_TNUMBER, encode_number_precision, 14), > + OPTION(LUA_TBOOLEAN, encode_load_metatables, 1), > + OPTION(LUA_TBOOLEAN, encode_use_tostring, 0), > + OPTION(LUA_TBOOLEAN, encode_invalid_as_nil, 0), > + OPTION(LUA_TBOOLEAN, decode_invalid_numbers, 1), > + OPTION(LUA_TBOOLEAN, decode_save_metatables, 1), > + OPTION(LUA_TNUMBER, decode_max_depth, 32), > + { NULL, 0, 0, 0}, > +}; > + > +int * > +parse_option(lua_State *L, int i, struct luaL_serializer* cfg) { > lua_getfield(L, 2, OPTIONS[i].name); > if (lua_isnil(L, -1)) { > lua_pop(L, 1); /* key hasn't changed */ > @@ -223,6 +239,28 @@ int* parse_option(lua_State *L, int i, struct luaL_serializer* cfg) { > return pval; > } > > +int > +parse_options(lua_State *L, struct luaL_serializer *cfg) { > + for (int i = 0; OPTIONS[i].name != NULL; i++) { > + int *pval = parse_option(L, i, cfg); > + /* Update struct luaL_serializer structure */ > + if (pval != NULL) > + lua_pop(L, 1); > + } > + lua_pop(l, 1); > + return 0; > +} > + > +/** > + * @brief serializer.cfg{} Lua binding for serializers. > + * serializer.cfg is a table that contains current configuration values from > + * luaL_serializer structure. serializer.cfg has overriden __call() method > + * to change configuration keys in internal userdata (like box.cfg{}). > + * Please note that direct change in serializer.cfg.key will not affect > + * internal state of userdata. > + * @param L lua stack > + * @return 0 > + */ > static int > luaL_serializer_cfg(lua_State *L) > { > @@ -244,8 +282,8 @@ luaL_serializer_cfg(lua_State *L) > default: > unreachable(); > } > - /* Save normalized value to serializer.cfg table */ > - lua_setfield(L, 1, OPTIONS[i].name); > + /* Save normalized value to serializer.cfg table */ > + lua_setfield(L, 1, OPTIONS[i].name); > } > } > return 0; > diff --git a/src/lua/utils.h b/src/lua/utils.h > index 4a2c3eaac..21eb36856 100644 > --- a/src/lua/utils.h > +++ b/src/lua/utils.h > @@ -1,6 +1,5 @@ > #ifndef TARANTOOL_LUA_UTILS_H_INCLUDED > #define TARANTOOL_LUA_UTILS_H_INCLUDED > -#pragma GCC diagnostic ignored "-Wunused-variable" > /* > * Copyright 2010-2015, Tarantool AUTHORS, please see AUTHORS file. > * > @@ -241,34 +240,29 @@ luaL_checkserializer(struct lua_State *L) { > luaL_checkudata(L, lua_upvalueindex(1), LUAL_SERIALIZER); > } > > -#define OPTION(type, name, defvalue) { #name, \ > - offsetof(struct luaL_serializer, name), type, defvalue} > /** > - * Configuration options for serializers > - * @sa struct luaL_serializer > + * parse_option is a function that is used to configure one field > + * in luaL_serializer struct. Adds one lua table to the top of > + * Lua stack. > + * @param L lua stack > + * @param index of option in OPTIONS[] > + * @param serializer to inherit configuration > + * @return ponter to the value of option > */ > -static struct { > - const char *name; > - size_t offset; /* offset in structure */ > - int type; > - int defvalue; > -} OPTIONS[] = { > - OPTION(LUA_TBOOLEAN, encode_sparse_convert, 1), > - OPTION(LUA_TNUMBER, encode_sparse_ratio, 2), > - OPTION(LUA_TNUMBER, encode_sparse_safe, 10), > - OPTION(LUA_TNUMBER, encode_max_depth, 32), > - OPTION(LUA_TBOOLEAN, encode_invalid_numbers, 1), > - OPTION(LUA_TNUMBER, encode_number_precision, 14), > - OPTION(LUA_TBOOLEAN, encode_load_metatables, 1), > - OPTION(LUA_TBOOLEAN, encode_use_tostring, 0), > - OPTION(LUA_TBOOLEAN, encode_invalid_as_nil, 0), > - OPTION(LUA_TBOOLEAN, decode_invalid_numbers, 1), > - OPTION(LUA_TBOOLEAN, decode_save_metatables, 1), > - OPTION(LUA_TNUMBER, decode_max_depth, 32), > - { NULL, 0, 0, 0}, > -}; > +int * > +parse_option(lua_State *l, int i, struct luaL_serializer* cfg); > > -int* parse_option(lua_State *l, int i, struct luaL_serializer* cfg); > +/** > + * parse_options is a function that is used to serialize lua table > + * of options to luaL_serializer struct. Removes the lua table from > + * the top of lua stack. > + * parse_options. > + * @param L lua stack > + * @param serializer to inherit configuration > + * @return 0 > + */ > +int > +parse_options(lua_State *l, struct luaL_serializer* cfg); > > /** A single value on the Lua stack. */ > struct luaL_field { > diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua > index 050d769ea..d76297ab5 100755 > --- a/test/app-tap/json.test.lua > +++ b/test/app-tap/json.test.lua > @@ -22,42 +22,55 @@ end > > tap.test("json", function(test) > local serializer = require('json') > - test:plan(18) > + test:plan(21) > > --- gh-2888 Added opions to encode(). > +-- gh-2888: check the possibility of using options in encode()/decode() > > local sub = {a = 1, { b = {c = 1, d = {e = 1}}}} > serializer.cfg({encode_max_depth = 1}) > test:ok(serializer.encode(sub) == '{"1":null,"a":1}', > - 'sub == {"1":null,"a":1}') > + 'depth of encoding is 1 with .cfg') > serializer.cfg({encode_max_depth = 2}) > test:ok(serializer.encode(sub) == '{"1":{"b":null},"a":1}', > - 'sub == {"1":{"b":null},"a":1}') > + 'depth of encoding is 2 with .cfg') > serializer.cfg({encode_max_depth = 2}) > - test:ok(serializer.encode(sub,{encode_max_depth = 1}) > - == '{"1":null,"a":1}', 'sub == {"1":null,"a":1}') > + test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}', > + 'depth of encoding is 1 with .encode') > > local nan = 1/0 > test:ok(serializer.encode({a = nan}) == '{"a":inf}', > - 'a = nan == {"a":inf}') > + 'default "encode_invalid_numbers"') > serializer.cfg({encode_invalid_numbers = false}) > test:ok(pcall(serializer.encode, {a = nan}) == false, > - 'error when "encode_invalid_numbers = false" with NaN') > + 'expected error with NaN ecoding with .cfg') > serializer.cfg({encode_invalid_numbers = true}) > test:ok(pcall(serializer.encode, {a = nan}, > - {encode_invalid_numbers = false}) == false, > - 'error when "encode_invalid_numbers = false" with NaN') > + {encode_invalid_numbers = false}) == false, > + 'expected error with NaN ecoding with .encode') > > local number = 0.12345 > test:ok(serializer.encode({a = number}) == '{"a":0.12345}', > - 'precision more than 5') > + 'precision more than 5') > serializer.cfg({encode_number_precision = 3}) > test:ok(serializer.encode({a = number}) == '{"a":0.123}', > - 'precision is 3') > + 'precision is 3') > serializer.cfg({encode_number_precision = 14}) > test:ok(serializer.encode({a = number}, > - {encode_number_precision = 3}) == '{"a":0.123}', 'precision is 3') > + {encode_number_precision = 3}) == '{"a":0.123}', 'precision is 3') > > + serializer.cfg({decode_invalid_numbers = false}) > + test:ok(pcall(serializer.decode, '{"a":inf}') == false, > + 'expected error with NaN decoding with .cfg') > + serializer.cfg({decode_invalid_numbers = true}) > + test:ok(pcall(serializer.decode, '{"a":inf}', > + {decode_invalid_numbers = false}) == false, > + 'expected error with NaN decoding with .decode') > + > + test:ok(pcall(serializer.decode, '{"1":{"b":{"c":1,"d":null}},"a":1}', > + {decode_max_depth = 2}) == false, > + 'error: too many nested data structures') > + > +-- > test:test("unsigned", common.test_unsigned, serializer) > test:test("signed", common.test_signed, serializer) > test:test("double", common.test_double, serializer) > diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c > index 861079f8a..29553fc4d 100644 > --- a/third_party/lua-cjson/lua_cjson.c > +++ b/third_party/lua-cjson/lua_cjson.c > @@ -417,21 +417,9 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg, > } > } > > -/*Serialize Lua table of options to luaL_serializer struct.*/ > -static int parse_options(lua_State *l, struct luaL_serializer* cfg) { > - for (int i = 0; OPTIONS[i].name != NULL; i++) { > - int *pval = parse_option(l, i, cfg); > - /* Update struct luaL_serializer structure */ > - if (pval != NULL) > - lua_pop(l, 1); > - } > - lua_pop(l, 1); > - return 0; > -} > - > static int json_encode(lua_State *l) { > luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), > - 1, "expected 1 or 2 arguments"); > + 1, "expected 1 or 2 arguments"); > > /* Reuse existing buffer */ > strbuf_reset(&encode_buf); > @@ -443,7 +431,7 @@ static int json_encode(lua_State *l) { > json_append_data(l, &user_cfg, 0, &encode_buf); > } else { > json_append_data(l, cfg, 0, &encode_buf); > -} > + } > > int len; > char *json = strbuf_string(&encode_buf, &len); > @@ -993,7 +981,7 @@ static int json_decode(lua_State *l) > size_t json_len; > > luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), > - 1, "expected 1 or 2 arguments"); > + 1, "expected 1 or 2 arguments"); > > if (lua_gettop(l) == 2) { > struct luaL_serializer user_cfg = *luaL_checkserializer(l); > > 26.07.2018, 00:35, "Vladislav Shpilevoy" : >> Hi! Thanks for the fixes! >> >> 1. Again, as I said on the previous review. Please, put a new >> patch version at the end of letter. >> >> 2. On the branch I still see the old version. So looks like >> you forgot to push. Please, do it and resend the patch.