[tarantool-patches] Re: [PATCH v3] json: add options to json.encode()

[roman.habibov1@yandex.ru]

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@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.