[tarantool-patches] Re: [PATCH v3] json: add options to json.encode()
roman.habibov1 at yandex.ru
roman.habibov1 at yandex.ru
Wed Aug 8 00:52:04 MSK 2018
Hi! Thanks for the review!
> 1. Put the branch onto 1.10, not on 2.0. It does not
> require SQL, so 1.10 is ok.
Done.
> 2. As I said earlier, please, use separate @retval for each
> possible value. One @retval for not NULL pointer, one @retval
> for NULL.
+/**
+ * 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)
> 3. Both here and in luaL_serializer_parse_options indentation
> is broken after you changed the functions names.
+luaL_serializer_parse_option(struct lua_State *L, int i,
+ struct luaL_serializer *cfg)
+void
+luaL_serializer_parse_options(struct lua_State *L,
+ struct luaL_serializer *cfg)
> 4. I've noticed that all the code below (push, setfield) is useless.
> parse_option gets the value from cfg[name] and sets it to struct
> luaL_cfg_serializer.name. Then the same value is pushed back onto
> the stack and set into cfg[name].
>
> So actually it looks like cfg[name] = cfg[name]. I've removed the
> code below and all works ok. Please, do it.
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);
- }
+ /* Iterate over all available options and checks keys in passed table. */
+ for (int i = 0; OPTIONS[i].name != NULL; i++)
+ luaL_serializer_parse_option(L, i, cfg);
return 0;
}
> 5. Please, align function's lines by the function's body offset. Including
> comments.
Fixed.
> 6. You've called the same cfg 2 lines above.
+ 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')
> 7. Please, align function arguments under the function beginning.
> I've pushed several fixes of this violation on a separate commit on
> your branch. Please, look at them, squash, and do the same alignment
> in other places in this file and other files.
Fixed.
> 8. I believe, instead of 'boolean == false' you can just do
> 'not boolean' (I've fixed it here. Do the same in other
> places).
+ test:ok(not pcall(serializer.encode, {a = nan}),
+ 'expected error with NaN ecoding with .cfg')
+ test:ok(not pcall(serializer.encode, {a = nan},
+ {encode_invalid_numbers = false}),
+ 'expected error with NaN ecoding with .encode')
+ test:ok(not pcall(serializer.decode, '{"a":inf}',
+ {decode_invalid_numbers = false}),
+ 'expected error with NaN decoding with .decode')
> 9. Garbage empty comment.
Deleted.
> 10. 'len' parameter is ok to be NULL here.
+ char *json = strbuf_string(&encode_buf, NULL);
+ lua_pushlstring(l, json, encode_buf.length);
commit f6d41bdfcec241733f98b7d26715ece6625539aa
Author: Roman Khabibov <roman.habibov1 at yandex.ru>
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.
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2f0f4dcf8..3faeb416f 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -214,6 +214,48 @@ 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);
+ lua_pop(L, 1);
+}
+
/**
* @brief serializer.cfg{} Lua binding for serializers.
* serializer.cfg is a table that contains current configuration values from
@@ -225,39 +267,14 @@ 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);
- }
+ /* Iterate over all available options and checks keys in passed table. */
+ for (int i = 0; OPTIONS[i].name != NULL; i++)
+ luaL_serializer_parse_option(L, i, cfg);
return 0;
}
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 6b057af3e..204bd8664 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..f2c67ab0a 100755
--- a/test/app-tap/json.test.lua
+++ b/test/app-tap/json.test.lua
@@ -22,7 +22,55 @@ 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..b175b770c 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -417,22 +417,24 @@ 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);
+ 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 +979,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;
More information about the Tarantool-patches
mailing list