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 CE91E29228 for ; Tue, 14 Aug 2018 18:29:41 -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 0bP-L1D4q5X0 for ; Tue, 14 Aug 2018 18:29:41 -0400 (EDT) Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 0F64229209 for ; Tue, 14 Aug 2018 18:29:40 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3] json: add options to json.encode() References: <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> <0e2c47c2-0276-a1da-c54b-726b64c17aae@tarantool.org> <5209131532608150@myt3-c7e5d17fe013.qloud-c.yandex.net> <2952461532611162@iva5-08e20335b0d9.qloud-c.yandex.net> <33ec689b-9aca-97e9-e56f-c0aee649ad87@tarantool.org> <28658371533050946@myt4-74a8acfc13eb.qloud-c.yandex.net> <898281533156060@myt5-cf6d29327892.qloud-c.yandex.net> <26831531533678724@myt5-f9d71769b752.qloud-c.yandex.net> <6304221534202083@iva8-37fc2ad204cd.qloud-c.yandex.net> From: Vladislav Shpilevoy Message-ID: Date: Wed, 15 Aug 2018 01:29:38 +0300 MIME-Version: 1.0 In-Reply-To: <6304221534202083@iva8-37fc2ad204cd.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" , Alexander Turenko Hi! Thanks for the fixes! I have force pushed one minor fix for alignment in lua_cjson.c. Alexander, please, make a second review. On 14/08/2018 02:14, roman.habibov1@yandex.ru wrote: > Hi! I also hope this is the last patch. > >> 2. As I've already said, please, start a sentence from a capital letter >> and finish with the dot. For example see other files: >> https://github.com/tarantool/tarantool/blob/2.0/src/box/tuple.h#L367 >> >> In other places too. > > +/** > + * Configure one field in @a cfg. > + * @param L Lua stack. > + * @param i Index of option in OPTIONS[]. > + * @param cfg Serializer to inherit configuration. > + * @retval Pointer to the value of option. > + * @retval NULL if option is not in the table. > + */ > > +/** > + * Parse configuration table into @a cfg. Remove the lua table > + * from the top of lua stack. > + * @param L Lua stack. > + * @param cfg Serializer to inherit configuration. > + */ > >> 3. From the previous review point 5 still is not fixed. Your comment >> starts earlier than the function. But should be aligned by the function >> body, like usual statement. (Add 4 spaces before the comment.) > > + -- gh-2888: Check the possibility of using options in encode()/decode(). > > commit ddb198cbc0eccb0fcc6c5479c295982db1aac1d2 > Author: Roman Khabibov > Date: Sun Jul 8 02:21:08 2018 +0300 > > json: add options to json.encode() > > Add an ability to pass options to json.encode()/decode(). > > Closes: #2888. > > @TarantoolBot document > Title: json.encode() json.decode() > Add an ability to pass options to > json.encode() and json.decode(). > These are the same options that > are used globally in json.cfg(). > > diff --git a/src/lua/utils.c b/src/lua/utils.c > index 2f0f4dcf8..b4115c6d2 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -214,6 +214,46 @@ static struct { > { NULL, 0, 0, 0}, > }; > > +/** > + * Configure one field in @a cfg. > + * @param L Lua stack. > + * @param i Index of option in OPTIONS[]. > + * @param cfg Serializer to inherit configuration. > + * @retval Pointer to the value of option. > + * @retval NULL if option is not in the table. > + */ > +static int * > +luaL_serializer_parse_option(struct 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); > + return NULL; > + } > + int *pval = (int *) ((char *) cfg + OPTIONS[i].offset); > + switch (OPTIONS[i].type) { > + case LUA_TBOOLEAN: > + *pval = lua_toboolean(L, -1); > + break; > + case LUA_TNUMBER: > + *pval = lua_tointeger(L, -1); > + break; > + default: > + unreachable(); > + } > + lua_pop(L, 1); > + return pval; > +} > + > +void > +luaL_serializer_parse_options(struct lua_State *L, > + struct luaL_serializer *cfg) > +{ > + for (int i = 0; OPTIONS[i].name != NULL; i++) > + luaL_serializer_parse_option(L, i, cfg); > +} > + > /** > * @brief serializer.cfg{} Lua binding for serializers. > * serializer.cfg is a table that contains current configuration values from > @@ -225,39 +265,11 @@ static struct { > * @return 0 > */ > static int > -luaL_serializer_cfg(lua_State *L) > +luaL_serializer_cfg(struct lua_State *L) > { > luaL_checktype(L, 1, LUA_TTABLE); /* serializer */ > luaL_checktype(L, 2, LUA_TTABLE); /* serializer.cfg */ > - struct luaL_serializer *cfg = luaL_checkserializer(L); > - /* Iterate over all available options and checks keys in passed table */ > - for (int i = 0; OPTIONS[i].name != NULL; i++) { > - lua_getfield(L, 2, OPTIONS[i].name); > - if (lua_isnil(L, -1)) { > - lua_pop(L, 1); /* key hasn't changed */ > - continue; > - } > - /* > - * Update struct luaL_serializer using pointer to a > - * configuration value (all values must be `int` for that). > - */ > - int *pval = (int *) ((char *) cfg + OPTIONS[i].offset); > - /* Update struct luaL_serializer structure */ > - switch (OPTIONS[i].type) { > - case LUA_TBOOLEAN: > - *pval = lua_toboolean(L, -1); > - lua_pushboolean(L, *pval); > - break; > - case LUA_TNUMBER: > - *pval = lua_tointeger(L, -1); > - lua_pushinteger(L, *pval); > - break; > - default: > - unreachable(); > - } > - /* Save normalized value to serializer.cfg table */ > - lua_setfield(L, 1, OPTIONS[i].name); > - } > + luaL_serializer_parse_options(L, luaL_checkserializer(L)); > return 0; > } > > diff --git a/src/lua/utils.h b/src/lua/utils.h > index 6b057af3e..9e2353511 100644 > --- a/src/lua/utils.h > +++ b/src/lua/utils.h > @@ -240,6 +240,16 @@ luaL_checkserializer(struct lua_State *L) { > luaL_checkudata(L, lua_upvalueindex(1), LUAL_SERIALIZER); > } > > +/** > + * Parse configuration table into @a cfg. Remove the lua table > + * from the top of lua stack. > + * @param L Lua stack. > + * @param cfg Serializer to inherit configuration. > + */ > +void > +luaL_serializer_parse_options(struct lua_State *l, > + struct luaL_serializer *cfg); > + > /** A single value on the Lua stack. */ > struct luaL_field { > union { > diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua > index 42c79d6e9..ce21bbfcf 100755 > --- a/test/app-tap/json.test.lua > +++ b/test/app-tap/json.test.lua > @@ -22,7 +22,53 @@ end > > tap.test("json", function(test) > local serializer = require('json') > - test:plan(13) > + test:plan(25) > + > + -- 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}', > + 'depth of encoding is 1 with .cfg') > + serializer.cfg({encode_max_depth = 2}) > + test:ok(serializer.encode(sub) == '{"1":{"b":null},"a":1}', > + 'depth of encoding is 2 with .cfg') > + 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}', > + 'default "encode_invalid_numbers"') > + serializer.cfg({encode_invalid_numbers = false}) > + test:ok(not pcall(serializer.encode, {a = nan}), > + 'expected error with NaN ecoding with .cfg') > + serializer.cfg({encode_invalid_numbers = true}) > + test:ok(not pcall(serializer.encode, {a = nan}, > + {encode_invalid_numbers = 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') > + serializer.cfg({encode_number_precision = 3}) > + test:ok(serializer.encode({a = number}) == '{"a":0.123}', > + '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') > + > + serializer.cfg({decode_invalid_numbers = false}) > + test:ok(not pcall(serializer.decode, '{"a":inf}'), > + 'expected error with NaN decoding with .cfg') > + serializer.cfg({decode_invalid_numbers = true}) > + test:ok(not pcall(serializer.decode, '{"a":inf}', > + {decode_invalid_numbers = false}), > + 'expected error with NaN decoding with .decode') > + > + test:ok(not pcall(serializer.decode, '{"1":{"b":{"c":1,"d":null}},"a":1}', > + {decode_max_depth = 2}), > + '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 11aa40225..e431c3fb0 100644 > --- a/third_party/lua-cjson/lua_cjson.c > +++ b/third_party/lua-cjson/lua_cjson.c > @@ -417,22 +417,25 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg, > } > } > > -static int json_encode(lua_State *l) > -{ > - struct luaL_serializer *cfg = luaL_checkserializer(l); > - char *json; > - int len; > +static int json_encode(lua_State *l) { > + luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), > + 1, "expected 1 or 2 arguments"); > > - luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument"); > - > - /* Reuse existing buffer */ > + /* Reuse existing buffer. */ > strbuf_reset(&encode_buf); > + struct luaL_serializer *cfg = luaL_checkserializer(l); > > - json_append_data(l, cfg, 0, &encode_buf); > - json = strbuf_string(&encode_buf, &len); > - > - lua_pushlstring(l, json, len); > + if (lua_gettop(l) == 2) { > + struct luaL_serializer user_cfg = *cfg; > + luaL_serializer_parse_options(l, &user_cfg); > + lua_pop(l, 1); > + json_append_data(l, &user_cfg, 0, &encode_buf); > + } else { > + json_append_data(l, cfg, 0, &encode_buf); > + } > > + char *json = strbuf_string(&encode_buf, NULL); > + lua_pushlstring(l, json, encode_buf.length); > return 1; > } > > @@ -977,9 +980,17 @@ static int json_decode(lua_State *l) > json_token_t token; > size_t json_len; > > - luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument"); > + luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), > + 1, "expected 1 or 2 arguments"); > + > + if (lua_gettop(l) == 2) { > + struct luaL_serializer user_cfg = *luaL_checkserializer(l); > + luaL_serializer_parse_options(l, &user_cfg); > + json.cfg = &user_cfg; > + } else { > + json.cfg = luaL_checkserializer(l); > + } > > - json.cfg = luaL_checkserializer(l); > json.data = luaL_checklstring(l, 1, &json_len); > json.current_depth = 0; > json.ptr = json.data; >