[tarantool-patches] Re: [PATCH v3] json: add options to json.encode()
roman.habibov1 at yandex.ru
roman.habibov1 at yandex.ru
Thu Jul 26 15:29:10 MSK 2018
Sorry again. I hurried.
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 3b89719ef..2f0f4dcf8 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -186,6 +186,7 @@ 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}
/**
@@ -213,44 +214,6 @@ static struct {
{ 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 */
- return NULL;
- }
- /*
- * 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);
- break;
- case LUA_TNUMBER:
- *pval = lua_tointeger(L, -1);
- break;
- default:
- unreachable();
- }
- 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
@@ -269,22 +232,31 @@ luaL_serializer_cfg(lua_State *L)
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++) {
- int* pval = parse_option(L, i, cfg);
+ 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 */
- if (pval != NULL) {
- switch (OPTIONS[i].type) {
- case LUA_TBOOLEAN:
- lua_pushboolean(L, *pval);
- break;
- case LUA_TNUMBER:
- lua_pushinteger(L, *pval);
- break;
- default:
- unreachable();
- }
+ 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);
- }
}
return 0;
}
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 21eb36856..6b057af3e 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -240,30 +240,6 @@ luaL_checkserializer(struct lua_State *L) {
luaL_checkudata(L, lua_upvalueindex(1), 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
- */
-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 {
union {
diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
index d76297ab5..3884b41e7 100755
--- a/test/app-tap/json.test.lua
+++ b/test/app-tap/json.test.lua
@@ -22,55 +22,7 @@ end
tap.test("json", function(test)
local serializer = require('json')
- test:plan(21)
-
--- 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')
- serializer.cfg({encode_max_depth = 2})
- 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(pcall(serializer.encode, {a = nan}) == false,
- '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,
- '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(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:plan(9)
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 29553fc4d..aa8217dfb 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -417,25 +417,22 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
}
}
-static int json_encode(lua_State *l) {
- luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1),
- 1, "expected 1 or 2 arguments");
+static int json_encode(lua_State *l)
+{
+ struct luaL_serializer *cfg = luaL_checkserializer(l);
+ char *json;
+ int len;
+
+ luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
/* Reuse existing buffer */
strbuf_reset(&encode_buf);
- struct luaL_serializer *cfg = luaL_checkserializer(l);
- if (lua_gettop(l) == 2) {
- struct luaL_serializer user_cfg = *cfg;
- parse_options(l, &user_cfg);
- json_append_data(l, &user_cfg, 0, &encode_buf);
- } else {
- json_append_data(l, cfg, 0, &encode_buf);
- }
+ json_append_data(l, cfg, 0, &encode_buf);
+ json = strbuf_string(&encode_buf, &len);
- int len;
- char *json = strbuf_string(&encode_buf, &len);
lua_pushlstring(l, json, len);
+
return 1;
}
@@ -980,17 +977,9 @@ static int json_decode(lua_State *l)
json_token_t token;
size_t json_len;
- 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);
- parse_options(l, &user_cfg);
- json.cfg = &user_cfg;
- } else {
- json.cfg = luaL_checkserializer(l);
- }
+ luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
+ json.cfg = luaL_checkserializer(l);
json.data = luaL_checklstring(l, 1, &json_len);
json.current_depth = 0;
json.ptr = json.data;
26.07.2018, 13:07, "Vladislav Shpilevoy" <v.shpilevoy at tarantool.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 at 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" <v.shpilevoy at tarantool.org>:
>>> 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.
More information about the Tarantool-patches
mailing list