[tarantool-patches] [PATCH] json: added options to json.encode()

[tarantool-patches] [PATCH] json: added options to json.encode()

[Roman Khabibov]

Added an ability to pass encoder options to json.encode().

Closes: #2888.
---
 test/app-tap/json.test.lua        | 27 +++++++++++-
 third_party/lua-cjson/lua_cjson.c | 90 +++++++++++++++++++++++++++++++++------
 2 files changed, 104 insertions(+), 13 deletions(-)

diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
index 3884b41..1252ad0 100755
--- a/test/app-tap/json.test.lua
+++ b/test/app-tap/json.test.lua
@@ -22,7 +22,32 @@ end
 
 tap.test("json", function(test)
     local serializer = require('json')
-    test:plan(9)
+    test:plan(18)
+
+-- gh-2888 Added opions to encode().
+
+    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}')
+    serializer.cfg({encode_max_depth = 2})
+    test:ok(serializer.encode(sub) == '{"1":{"b":null},"a":1}', 'sub == {"1":{"b":null},"a":1}')
+    serializer.cfg({encode_max_depth = 2})
+    test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}', 'sub == {"1":null,"a":1}')
+
+    local nan = 1/0
+    test:ok(serializer.encode({a = nan}) == '{"a":inf}', 'a = nan == {"a":inf}')
+    serializer.cfg({encode_invalid_numbers = false})
+    test:ok(pcall(serializer.encode, {a = nan}) == false, 'error when "encode_invalid_numbers = false" with NaN')
+    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')
+
+    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')
+
     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 aa8217d..bdfda1e 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -417,23 +417,89 @@ 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;
+#define OPTION(type, name, defvalue) { #name, \
+	offsetof(struct luaL_serializer, name), type, defvalue}
+
+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},
+};
 
-    luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
+/*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++) {
+		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_pop(l, 1);
+			break;
+		case LUA_TNUMBER:
+			*pval = lua_tointeger(l, -1);
+			lua_pop(l, 1);
+			break;
+		default:
+			unreachable();
+		}
+	}
+	lua_pop(l, 1);
+	return 0;
+}
 
-    /* Reuse existing buffer */
-    strbuf_reset(&encode_buf);
+static int json_encode(lua_State *l) {
+	luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), 1, "expected 1 or 2 arguments");
 
-    json_append_data(l, cfg, 0, &encode_buf);
-    json = strbuf_string(&encode_buf, &len);
+	char *json;
+	int len;
 
-    lua_pushlstring(l, json, len);
+	/* Reuse existing buffer */
+	strbuf_reset(&encode_buf);
+	struct luaL_serializer *cfg = luaL_checkserializer(l);
 
-    return 1;
+	/* If have second arg */
+	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);
+	}
+	/* If have not second arg */
+	else {
+
+		json_append_data(l, cfg, 0, &encode_buf);
+	}
+
+	json = strbuf_string(&encode_buf, &len);
+	lua_pushlstring(l, json, len);
+
+	return 1;
 }
 
 /* ===== DECODING ===== */
-- 
2.7.4

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

[Vladislav Shpilevoy]

Hello. Thanks for the patch! My congratulations on your
second patch!

See and fix 10 comments below to make your patch better.


1. Please, use imperative in commit header.

Not 'json: added options to json.encode()', but
     'json: add options to json.encode()'.


On 08/07/2018 03:57, Roman Khabibov wrote:
> Added an ability to pass encoder options to json.encode().
> 
> Closes: #2888.
> ---
>   test/app-tap/json.test.lua        | 27 +++++++++++-
>   third_party/lua-cjson/lua_cjson.c | 90 +++++++++++++++++++++++++++++++++------
>   2 files changed, 104 insertions(+), 13 deletions(-)
> 
> diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
> index 3884b41..1252ad0 100755
> --- a/test/app-tap/json.test.lua
> +++ b/test/app-tap/json.test.lua

2. I will review tests later when we will finish with the other code.

> diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
> index aa8217d..bdfda1e 100644
> --- a/third_party/lua-cjson/lua_cjson.c
> +++ b/third_party/lua-cjson/lua_cjson.c
> @@ -417,23 +417,89 @@ 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;
> +#define OPTION(type, name, defvalue) { #name, \
> +	offsetof(struct luaL_serializer, name), type, defvalue}

3. All this code including parse_options is a full copy of
the code from src/lua/util.c. Please, find a way how to do not
duplicate the code. You should consolidate an options parser
function from util.c code and make a declaration in a header.
And then use this function both in util.c in luaL_serializer_cfg()
and in json_encode().

> +static int json_encode(lua_State *l) {
> +	luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), 1, "expected 1 or 2 arguments");

4. Out of 80 symbols.

>   
> -    json_append_data(l, cfg, 0, &encode_buf);
> -    json = strbuf_string(&encode_buf, &len);
> +	char *json;
> +	int len;

5. In the original code spaces were used, not tabs. Please,
use spaces in this file too.

6. You do not need to annotate those arguments here. Please,
declare them together with assigning.

Not
var name;
name = value;

But
var name = value.

>   
> -    lua_pushlstring(l, json, len);
> +	/* Reuse existing buffer */
> +	strbuf_reset(&encode_buf);
> +	struct luaL_serializer *cfg = luaL_checkserializer(l);
>   
> -    return 1;
> +	/* If have second arg */
> +	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);
> +	}
> +	/* If have not second arg */
> +	else {

7. Please, put 'else' on the same line as }.

8. Put the dot at the end of sentence.

> +

9. Extra empty line.

> +		json_append_data(l, cfg, 0, &encode_buf);
> +	}
> +
> +	json = strbuf_string(&encode_buf, &len);
> +	lua_pushlstring(l, json, len);
> +
> +	return 1;
>   }

10. Please, add the options to json.decode. API must be symmetrical.

>   
>   /* ===== DECODING ===== */
> 

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

[roman.habibov1]

> 1. Please, use imperative in commit header.

Fixed.


>>  diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
>>  index aa8217d..bdfda1e 100644
>>  --- a/third_party/lua-cjson/lua_cjson.c
>>  +++ b/third_party/lua-cjson/lua_cjson.c
>>  @@ -417,23 +417,89 @@ 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;
>>  +#define OPTION(type, name, defvalue) { #name, \
>>  + offsetof(struct luaL_serializer, name), type, defvalue}
>
> 3. All this code including parse_options is a full copy of
> the code from src/lua/util.c. Please, find a way how to do not
> duplicate the code. You should consolidate an options parser
> function from util.c code and make a declaration in a header.
> And then use this function both in util.c in luaL_serializer_cfg()
> and in json_encode().
>
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2f0f4dcf8..0626bf76b 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -186,34 +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}
-/**
- * Configuration options for serializers
- * @sa struct luaL_serializer
- */
-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},
-};
-
 /**
  * @brief serializer.cfg{} Lua binding for serializers.
  * serializer.cfg is a table that contains current configuration values from
@@ -224,6 +196,33 @@ static struct {
  * @param L lua stack
  * @return 0
  */
+
+/*Serialize one option. Returns ponter to the value of option.*/
+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;
+}
+
 static int
 luaL_serializer_cfg(lua_State *L)
 {
@@ -232,31 +231,22 @@ 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++) {
-		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);
+		int* pval = parse_option(L, i, cfg);
 		/* 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();
+		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();
+			}
+			/* 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 6b057af3e..4a2c3eaac 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -1,5 +1,6 @@
 #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.
  *
@@ -240,6 +241,35 @@ 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
+ */
+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);
+
 /** A single value on the Lua stack. */
 struct luaL_field {
 	union {
diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index bdfda1e79..861079f8a 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -417,89 +417,38 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
     }
 }
 
-#define OPTION(type, name, defvalue) { #name, \
-	offsetof(struct luaL_serializer, name), type, defvalue}
-
-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},
-};
-
-/*Serialize Lua table of options to luaL_serializer struct*/
+/*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++) {
-		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_pop(l, 1);
-			break;
-		case LUA_TNUMBER:
-			*pval = lua_tointeger(l, -1);
-			lua_pop(l, 1);
-			break;
-		default:
-			unreachable();
-		}
-	}
-	lua_pop(l, 1);
-	return 0;
+        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");
>
> 4. Out of 80 symbols.
>

diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index bdfda1e79..861079f8a 100644
-	luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1), 1, "expected 1 or 2 arguments");
+    luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1),
+                      1, "expected 1 or 2 arguments");

>>  - json_append_data(l, cfg, 0, &encode_buf);
>>  - json = strbuf_string(&encode_buf, &len);
>>  + char *json;
>>  + int len;
>
> 5. In the original code spaces were used, not tabs. Please,
> use spaces in this file too.
>
> 6. You do not need to annotate those arguments here. Please,
> declare them together with assigning.
>
> Not
> var name;
> name = value;
>
> But
> var name = value.
>

diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index bdfda1e79..861079f8a 100644
-	char *json;
-	int len;
+    int len;
+    char *json = strbuf_string(&encode_buf, &len);

>>  - lua_pushlstring(l, json, len);
>>  + /* Reuse existing buffer */
>>  + strbuf_reset(&encode_buf);
>>  + struct luaL_serializer *cfg = luaL_checkserializer(l);
>>
>>  - return 1;
>>  + /* If have second arg */
>>  + 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);
>>  + }
>>  + /* If have not second arg */
>>  + else {
>
> 7. Please, put 'else' on the same line as }.

Fixed.

> 8. Put the dot at the end of sentence.
>
>>  +

Fixed.

> 9. Extra empty line.
>
>>  + json_append_data(l, cfg, 0, &encode_buf);
>>  + }
>>  +
>>  + json = strbuf_string(&encode_buf, &len);
>>  + lua_pushlstring(l, json, len);
>>  +
>>  + return 1;
>>    }

Fixed.

> 10. Please, add the options to json.decode. API must be symmetrical.


diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index bdfda1e79..861079f8a 100644
 /* ===== DECODING ===== */
@@ -1043,9 +992,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);
+        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;

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

[Vladislav Shpilevoy]

Hi! Thanks for the fixes! Please, put a new version of
the patch at the end of email sending the former. Now I
did it for you.

See 10 comments below.

> commit a229af626a78c7f37b628101e189f7727c01ccd8
> Author: Roman Khabibov <roman.habibov1@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..0626bf76b 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -186,34 +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}

1. You should not touch this hunk. Utils should provide
only API to configure the serializer, and do not expose
options array.

> -/**
> - * Configuration options for serializers
> - * @sa struct luaL_serializer
> - */
> -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},
> -};
> -
>  /**
>   * @brief serializer.cfg{} Lua binding for serializers.
>   * serializer.cfg is a table that contains current configuration values from
> @@ -224,6 +196,33 @@ static struct {
>   * @param L lua stack
>   * @return 0
>   */

2. You've cut the comment off. It belongs to another function.

> +
> +/*Serialize one option. Returns ponter to the value of option.*/
> +int* parse_option(lua_State *L, int i, struct luaL_serializer* cfg) {

3. Bad style. Please, see in other functions how to write comments
and how to declare pointers. You should not write int*, but int *.
Please, wrap line after the function return type declaring.

> +	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;
> +}
> +
>  static int
>  luaL_serializer_cfg(lua_State *L)
>  {
> @@ -232,31 +231,22 @@ 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++) {
> -		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);
> +		int* pval = parse_option(L, i, cfg);
>  		/* 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();
> +		if (pval != NULL) {
> +			switch (OPTIONS[i].type) {
> +			case LUA_TBOOLEAN:
> +					lua_pushboolean(L, *pval);

4. Broken indentation.

> +				break;
> +			case LUA_TNUMBER:
> +				lua_pushinteger(L, *pval);
> +				break;
> +			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 6b057af3e..4a2c3eaac 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -1,5 +1,6 @@
>  #ifndef TARANTOOL_LUA_UTILS_H_INCLUDED
>  #define TARANTOOL_LUA_UTILS_H_INCLUDED
> +#pragma GCC diagnostic ignored "-Wunused-variable"

5. Garbage diff. You should never change compilation
options via pragma. And if you need this one, then your
patch has a problem, that should be fixed, not hidden.

>  /*
>   * Copyright 2010-2015, Tarantool AUTHORS, please see AUTHORS file.
>   *
> @@ -240,6 +241,35 @@ 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
> + */
> +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);

6. Please, write function comment on top of the declaration. Not on
top of the implementation.

> +
>  /** 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 3884b41e7..050d769ea 100755
> --- a/test/app-tap/json.test.lua
> +++ b/test/app-tap/json.test.lua
> @@ -22,7 +22,42 @@ end
>  
>  tap.test("json", function(test)
>      local serializer = require('json')
> -    test:plan(9)
> +    test:plan(18)
> +
> +-- gh-2888 Added opions to encode().

7. Typo: opions.

8. Please, consult other test files how to write a test comment.
It should not be a description of what was done, but an explanation
of the problem.

9. I do not see tests on decode.

> diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
> index aa8217dfb..861079f8a 100644
> --- a/third_party/lua-cjson/lua_cjson.c
> +++ b/third_party/lua-cjson/lua_cjson.c
> @@ -417,22 +417,37 @@ 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;
> +/*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;
> +}
>  
> -    luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
> +static int json_encode(lua_State *l) {
> +    luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1),
> +                      1, "expected 1 or 2 arguments");

10. Bad indentation. In decode too.

>  
>      /* 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);
> +    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);
> +}
>  
> +    int len;
> +    char *json = strbuf_string(&encode_buf, &len);
>      lua_pushlstring(l, json, len);
> -
>      return 1;
>  }
>  

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

[roman.habibov1]

Hi, thanks for your review.

Branch: https://github.com/tarantool/tarantool/tree/romankhabibov/gh-2888-options-to-encode

Issue: https://github.com/tarantool/tarantool/issues/2888


> 1. You should not touch this hunk. Utils should provide
> only API to configure the serializer, and do not expose
> options array.
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},
+};

> 2. You've cut the comment off. It belongs to another function.
Fixed.

> 3. Bad style. Please, see in other functions how to write comments
> and how to declare pointers. You should not write int*, but int *.
> Please, wrap line after the function return type declaring.
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
 /**
- * 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
  */
+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);

> 4. Broken indentation.
Fixed.

> 5. Garbage diff. You should never change compilation
> options via pragma. And if you need this one, then your
> patch has a problem, that should be fixed, not hidden.
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"

> 6. Please, write function comment on top of the declaration. Not on
> top of the implementation.
Fixed.

> 7. Typo: opions.
Fixed.

> 8. Please, consult other test files how to write a test comment.
> It should not be a description of what was done, but an explanation
> of the problem.
> 9. I do not see tests on decode.
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()
+    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')
+
+--



> 10. Bad indentation. In decode too.
Fixed.

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

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

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

[roman.habibov1]

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.

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

[Vladislav Shpilevoy]

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

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

[roman.habibov1]

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

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

[Vladislav Shpilevoy]

The diff below again differs from the one on the branch.
Here what I see on the branch: https://github.com/tarantool/tarantool/commit/1476db8c67470f149e31f67255131802a7e02690

Its first diff is removal of a line from src/lua/utils.c. But diff
below firstly adds the line. Looks like you have sent reverted diff.

On 26/07/2018 15:29, roman.habibov1@yandex.ru wrote:
> 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@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@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@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.

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

[roman.habibov1]

diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2f0f4dcf8..3b89719ef 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 {
 	{ 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
@@ -232,31 +269,22 @@ 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++) {
-		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);
+		int* pval = parse_option(L, i, cfg);
 		/* 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();
-		}
+		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();
+			}
 		/* 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 6b057af3e..21eb36856 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -240,6 +240,30 @@ 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 3884b41e7..d76297ab5 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(9)
+    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: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 aa8217dfb..29553fc4d 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;
-
-    luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
+static int json_encode(lua_State *l) {
+    luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1),
+        1, "expected 1 or 2 arguments");
 
     /* 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);
+    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);
+    }
 
+    int len;
+    char *json = strbuf_string(&encode_buf, &len);
     lua_pushlstring(l, json, len);
-
     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);
+        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;

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

[Vladislav Shpilevoy]

Hi! Thanks for working on the patch!

On 26/07/2018 16:19, roman.habibov1@yandex.ru wrote:
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 2f0f4dcf8..3b89719ef 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);
>   }
>   
> -

1. Unnecessary diff. Please, avoid making additional
non-functional changes which increase number of hunks
in diff.

>   #define OPTION(type, name, defvalue) { #name, \
>   	offsetof(struct luaL_serializer, name), type, defvalue}
>   /**
> @@ -214,6 +213,44 @@ static struct {
>   	{ NULL, 0, 0, 0},
>   };
>   
> +int *
> +parse_option(lua_State *L, int i, struct luaL_serializer* cfg) {

2. This function is not used out of utils.c and can be
declared as static and removed from the header.

3. Wrong style of luaL_serializer pointer declaration.
Please, put '*' next to the name and after whitespace
next to the type.

4. Please, do not omit 'struct' when declare struct
variables. Here I am pointing out lua_State parameter.

> +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 */

5. Below this line you do not update anything. What do you
mean?

6. Why do you need to announce pval? Why not just

     if (parse_option(L, i, cfg) != NULL)

?

> +        if (pval != NULL)
> +            lua_pop(L, 1);
> +    }
> +    lua_pop(L, 1);
> +    return 0;
> +}

7. Please, use 8-width tabs instead of spaces in this
file.

> +
>   /**
>    * @brief serializer.cfg{} Lua binding for serializers.
>    * serializer.cfg is a table that contains current configuration values from
> @@ -232,31 +269,22 @@ 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++) {
> -		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);
> +		int* pval = parse_option(L, i, cfg);

8. Please, fix pointers declaration style. The previous
was correct.

>   		/* 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();
> -		}
> +		if (pval != NULL) {
> +			switch (OPTIONS[i].type) {
> +			case LUA_TBOOLEAN:
> +					lua_pushboolean(L, *pval);

9. Broken indentation.

> +				break;
> +			case LUA_TNUMBER:
> +				lua_pushinteger(L, *pval);
> +				break;
> +			default:
> +				unreachable();
> +			}
>   		/* Save normalized value to serializer.cfg table */
>   		lua_setfield(L, 1, OPTIONS[i].name);

10. Same.

> +		}
>   	}
>   	return 0;
>   }
> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index 6b057af3e..21eb36856 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -240,6 +240,30 @@ 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.

11. Please, consult other places how to write function comments.
In a comment you should use imperative (like in the commit title)
and it is not mandatory thing to either duplicate function name
in the comment or write things like 'This function does blah-blah'.

Just write something like 'Parse configuration table into @a cfg ...'.

> + * @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 3884b41e7..d76297ab5 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(9)
> +    test:plan(21)
> +
> +-- gh-2888: check the possibility of using options in encode()/decode()

12. Please, finish the sentence with the dot. Yes, even
in the tests.

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

[roman.habibov1]

Thanks for review.

> 1. Unnecessary diff. Please, avoid making additional
> non-functional changes which increase number of hunks
> in diff.
Fixed.


> 2. This function is not used out of utils.c and can be
> declared as static and removed from the header.
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 6b057af3e..2faa966fe 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.
+ * parse_options.
+ * @param L lua stack
+ * @param cfg serializer to inherit configuration
+ */
+void
+parse_options(struct lua_State *l, struct luaL_serializer *cfg);
+
 /** A single value on the Lua stack. */
 struct luaL_field {
 	union {


> 3. Wrong style of luaL_serializer pointer declaration.
> Please, put '*' next to the name and after whitespace
> next to the type.
Fixed.


> 4. Please, do not omit 'struct' when declare struct
> variables. Here I am pointing out lua_State parameter.
Fixed in utils.c, but not in lua-cjson.c.


> 6. Why do you need to announce pval? Why not just
>
>      if (parse_option(L, i, cfg) != NULL)
>
> ?
>
+void
+parse_options(struct lua_State *L, struct luaL_serializer *cfg) {
+	for (int i = 0; OPTIONS[i].name != NULL; i++) {
+		if (parse_option(L, i, cfg) != NULL)
+			lua_pop(L, 1);
+	}
+	lua_pop(L, 1);
+}


> 7. Please, use 8-width tabs instead of spaces in this
> file.
Fixed.


> 8. Please, fix pointers declaration style. The previous
> was correct.
Fixed.


> 9. Broken indentation.
> 10. Same.
Fixed.


> 11. Please, consult other places how to write function comments.
> In a comment you should use imperative (like in the commit title)
> and it is not mandatory thing to either duplicate function name
> in the comment or write things like 'This function does blah-blah'.
+/**
+ * Configure one field in @a cfg. Add one lua table to the top of
+ * lua stack.
+ * @param L lua stack
+ * @param i index of option in OPTIONS[]
+ * @param cfg serializer to inherit configuration
+ * @return ponter to the value of option, NULL if option is not
+ * in the table
+ */
+static int *
+parse_option(struct lua_State *L, int i, struct luaL_serializer *cfg) {

+/**
+ * Parse configuration table into @a cfg. Remove the lua table
+ * from the top of lua stack.
+ * parse_options.
+ * @param L lua stack
+ * @param cfg serializer to inherit configuration
+ */
+void
+parse_options(struct lua_State *l, struct luaL_serializer *cfg);
+


> 12. Please, finish the sentence with the dot. Yes, even
> in the tests.
+-- gh-2888: Check the possibility of using options in encode()/decode().


commit ebef41bb68abbac75ed2d7cb4a5fa65d282f4ee3
Author: Roman Khabibov <roman.habibov1@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..12fa8ff0d 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -214,6 +214,50 @@ static struct {
 	{ NULL, 0, 0, 0},
 };
 
+/**
+ * Configure one field in @a cfg. Add one lua table to the top of
+ * lua stack.
+ * @param L lua stack
+ * @param i index of option in OPTIONS[]
+ * @param cfg serializer to inherit configuration
+ * @return ponter to the value of option, NULL if option is not
+ * in the table
+ */
+static int *
+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); /* 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;
+}
+
+void
+parse_options(struct lua_State *L, struct luaL_serializer *cfg) {
+	for (int i = 0; OPTIONS[i].name != NULL; i++) {
+		if (parse_option(L, i, cfg) != NULL)
+			lua_pop(L, 1);
+	}
+	lua_pop(L, 1);
+}
+
 /**
  * @brief serializer.cfg{} Lua binding for serializers.
  * serializer.cfg is a table that contains current configuration values from
@@ -225,38 +269,29 @@ 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);
+		int *pval = parse_option(L, i, cfg);
 		/* 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();
+		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();
+			}
+			/* 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 6b057af3e..2faa966fe 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.
+ * parse_options.
+ * @param L lua stack
+ * @param cfg serializer to inherit configuration
+ */
+void
+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 3884b41e7..2a219ec24 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(9)
+    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: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 aa8217dfb..29553fc4d 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;
-
-    luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
+static int json_encode(lua_State *l) {
+    luaL_argcheck(l, (lua_gettop(l) == 2) || (lua_gettop(l) == 1),
+        1, "expected 1 or 2 arguments");
 
     /* 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);
+    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);
+    }
 
+    int len;
+    char *json = strbuf_string(&encode_buf, &len);
     lua_pushlstring(l, json, len);
-
     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);
+        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;

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

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

See 8 comments.

> commit ebef41bb68abbac75ed2d7cb4a5fa65d282f4ee3
> Author: Roman Khabibov <roman.habibov1@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..12fa8ff0d 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -214,6 +214,50 @@ static struct {
>   	{ NULL, 0, 0, 0},
>   };
>   
> +/**
> + * Configure one field in @a cfg. Add one lua table to the top of
> + * lua stack.

1. Looks like the comment is deceptive. When the function returns
NULL, it pops the value and has no after effects. When it returns
not NULL, it pushes not table but integer. Please, rewrite the
comment more carefully.

2. This function as well as parse_options is method of luaL_serializer,
so it should has prefix luaL_serializer_ like luaL_serializer_cfg.

     luaL_serializer_parse_option
     luaL_serializer_parse_options

> + * @param L lua stack
> + * @param i index of option in OPTIONS[]
> + * @param cfg serializer to inherit configuration
> + * @return ponter to the value of option, NULL if option is not

3. Typo: 'ponter'.

4. For different retvals use separate @retval lines.

5. Use @retval instead of @return, as I said you in the
private chat.

> + * in the table
> + */
> +static int *
> +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); /* key hasn't changed */

6. Write comments on separate lines, start with capital letter,
finish with dot.

> +		return NULL;
> +	}

7. Lets always pop the value you got above before returning.
It is a strange behavior when a function sometimes pushes onto
the stack, and sometimes not. And I've found a bug caused by
this ambiguity. See the next comment.

> +	/*
> +	 * 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;
> +}
> +
> +void
> +parse_options(struct lua_State *L, struct luaL_serializer *cfg) {
> +	for (int i = 0; OPTIONS[i].name != NULL; i++) {
> +		if (parse_option(L, i, cfg) != NULL)
> +			lua_pop(L, 1);
> +	}
> +	lua_pop(L, 1);
> +}
> +
>   /**
>    * @brief serializer.cfg{} Lua binding for serializers.
>    * serializer.cfg is a table that contains current configuration values from
> @@ -225,38 +269,29 @@ 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);
> +		int *pval = parse_option(L, i, cfg);
>   		/* 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();
> +		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();
> +			}
> +			/* Save normalized value to serializer.cfg table */
> +			lua_setfield(L, 1, OPTIONS[i].name);
>   		}

8. Here you have a bug - the lua stack always grows. It was even
before your patch.

Before parse_option above you have stack size N. After parse_option
is called and returned not NULL you have stack size N + 1. After
lua_pushboolean/pushinteger you have stack size N + 2. After
lua_setfield it becomes N + 1. So on each iteration of the cycle
the stack grows by 1. It should not.

> -		/* Save normalized value to serializer.cfg table */
> -		lua_setfield(L, 1, OPTIONS[i].name);
>   	}
>   	return 0;
>   }

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

[roman.habibov1]

Hi! Thanks for review.


> 1. Looks like the comment is deceptive. When the function returns
> NULL, it pops the value and has no after effects. When it returns
> not NULL, it pushes not table but integer. Please, rewrite the
> comment more carefully.
+/**
+ * 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, NULL if option is not
+ * in the table
+ */



> 2. This function as well as parse_options is method of luaL_serializer,
> so it should has prefix luaL_serializer_ like luaL_serializer_cfg.
>
>      luaL_serializer_parse_option
>      luaL_serializer_parse_options
+static int *
+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)


> 3. Typo: 'ponter'.
> 4. For different retvals use separate @retval lines.
> 5. Use @retval instead of @return, as I said you in the
> private chat.
+ * @retval pointer to the value of option, NULL if option is not


> 6. Write comments on separate lines, start with capital letter,
> finish with dot.
I deleted the comments in my functions, because this is not my comments.


> 7. Lets always pop the value you got above before returning.
> It is a strange behavior when a function sometimes pushes onto
> the stack, and sometimes not. And I've found a bug caused by
> this ambiguity. See the next comment.
> 8. Here you have a bug - the lua stack always grows. It was even
> before your patch.
>
> Before parse_option above you have stack size N. After parse_option
> is called and returned not NULL you have stack size N + 1. After
> lua_pushboolean/pushinteger you have stack size N + 2. After
> lua_setfield it becomes N + 1. So on each iteration of the cycle
> the stack grows by 1. It should not.
Now after calling luaL_serializer_parse_option, there are always N elements on the stack.


commit 1ecdfa05d8aa0037da79eeeba19cecb06bab2103
Author: Roman Khabibov <roman.habibov1@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..e200fee0a 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -214,6 +214,47 @@ 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, 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,38 +266,29 @@ 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 */
+	/* 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();
+		int *pval = luaL_serializer_parse_option(L, i, cfg);
+		/* 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();
+			}
+			/* 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 6b057af3e..57c72eb8b 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 3884b41e7..2a219ec24 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(9)
+    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: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 aa8217dfb..1a1bb3180 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);
+    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);
+    }
 
+    int len;
+    char *json = strbuf_string(&encode_buf, &len);
     lua_pushlstring(l, json, len);
-
     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;

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

[Vladislav Shpilevoy]

Hi! Thanks for the fixes! See 10 comments below.

1. Put the branch onto 1.10, not on 2.0. It does not
require SQL, so 1.10 is ok.

> commit 1ecdfa05d8aa0037da79eeeba19cecb06bab2103
> Author: Roman Khabibov <roman.habibov1@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..e200fee0a 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -214,6 +214,47 @@ 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, NULL if option is not
> + * in the table

2. As I said earlier, please, use separate @retval for each
possible value. One @retval for not NULL pointer, one @retval
for NULL.

> + */
> +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.

> +{
> +	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,38 +266,29 @@ 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 */
> +	/* 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();
> +		int *pval = luaL_serializer_parse_option(L, i, 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.

> +		/* 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();
> +			}
> +			/* 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/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
> index 3884b41e7..2a219ec24 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(9)
> +    test:plan(21)
> +
> +-- gh-2888: Check the possibility of using options in encode()/decode().

5. Please, align function's lines by the function's body offset. Including
comments.

> +
> +    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})

6. You've called the same cfg 2 lines above.

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

> +
> +    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,

8. I believe, instead of 'boolean == false' you can just do
'not boolean' (I've fixed it here. Do the same in other
places).

> +        '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')
> +
> +--

9. Garbage empty comment.

>       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 aa8217dfb..1a1bb3180 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);
> +    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);
> +    }
>   
> +    int len;
> +    char *json = strbuf_string(&encode_buf, &len);

10. 'len' parameter is ok to be NULL here.

>       lua_pushlstring(l, json, len);
> -
>       return 1;
>   }

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

[roman.habibov1]

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

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

[roman.habibov1]

Addition.
New branch: https://github.com/tarantool/tarantool/tree/romankhabibov/gh-2888-options-encode

08.08.2018, 00:52, "roman.habibov1@yandex.ru" <roman.habibov1@yandex.ru>:
> 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@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;

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

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

See below last (I hope) 3 comments.

> commit f6d41bdfcec241733f98b7d26715ece6625539aa
> Author: Roman Khabibov <roman.habibov1@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.

1. Put here a documentation bot request. We have a bot that tracks
all new commits and when in a one it sees a documentation request,
and the commit is pushed into the master, the bot opens an issue
in tarantool/doc repository. So the documentation writers team is
able to reflect your changes on site tarantool.io.

Your patch has changed public API and requires such documentation
request. For syntax see this:
https://github.com/tarantool/docbot#docbot---tarantool-documentation-pipeline-bot

For example of a request see this:
https://www.freelists.org/post/tarantool-patches/PATCH-78-box-introduce-boxctlpromote
where "@TarantoolBot document" is written.

For example of a result see this:
https://github.com/tarantool/doc/issues/created_by/TarantoolBot

Here you can check was your request successful or not (by syntax, for
instance): http://try.tarantool.org:11116

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

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.

> 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().

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

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

[roman.habibov1]

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 <roman.habibov1@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.
    
    @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;

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

[Vladislav Shpilevoy]

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 <roman.habibov1@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.
>      
>      @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;
> 

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

[Alexander Turenko]

Hi!

I'll make the review on the next week. Sorry for the late response.

WBR, Alexander Turenko.

On Wed, Aug 15, 2018 at 01:29:38AM +0300, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes! I have force pushed one minor
> fix for alignment in lua_cjson.c.
> 
> Alexander, please, make a second review.
> 

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

[Alexander Turenko]

Hi, Roman!

Sorry for the late response.

Please, consider inline comments.

WBR, Alexander Turenko.

On Wed, Aug 15, 2018 at 01:29:38AM +0300, Vladislav Shpilevoy wrote:
> 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:

> > -		/*
> > -		 * Update struct luaL_serializer using pointer to a
> > -		 * configuration value (all values must be `int` for that).
> > -		 */

Why this comment was stripped away? It seems to be still relevant to the
new code.

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

It seems it does not remove the table from a stack.

> > 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}}}}

Proposed to save encode_max_depth default value here, like so:

local orig_encode_max_depth = serializer.cfg.encode_max_depth

> > +    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')

And restore it here:

serializer.cfg({encode_max_depth = orig_encode_max_depth})

BTW, I think test case with {encode_max_depth = 2} is redundant.

> > +    test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}',
> > +            'depth of encoding is 1 with .encode')
> > +

Here we can check that json.encode(data, opts) did not change the global
configuration:

test:is(serializer.cfg.encode_max_depth, orig_encode_max_depth,
    'global option remains unchanged')

The same comments are applicable to other test cases.

> > +    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')
> > +

ecoding -> encoding (two occurences)

> > +    test:ok(not pcall(serializer.decode, '{"1":{"b":{"c":1,"d":null}},"a":1}',
> > +                      {decode_max_depth = 2}),
> > +            'error: too many nested data structures')
> > +

It is not obvious that gh-2888 block ends here. Please, add appropriate
name for the block below or move gh-2888 block below the unnamed block.

Look also how gh-3514 block header is formatted. It would be good to
have all headers in the same style.

> > 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);

lua_pop is used here (I guessto remove the table with options from a
stack), but don't used in decode.

> > +        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);

encode_buf.length breaks incapsulation of strbuf 'object'. Please, use
strbuf_length function instead.

> >       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);

lua_pop is not used here, but used in encode.

> > +        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;
> > 

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

[roman.habibov1]

Hi! Thanks for review.

>>  > - /*
>>  > - * Update struct luaL_serializer using pointer to a
>>  > - * configuration value (all values must be `int` for that).
>>  > - */
>
> Why this comment was stripped away? It seems to be still relevant to the
> new code.
Returned.

>>  > 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.
>
> It seems it does not remove the table from a stack.
Yes. Sentence deleted.

>>  > 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}}}}
>
> Proposed to save encode_max_depth default value here, like so:
>
> local orig_encode_max_depth = serializer.cfg.encode_max_depth
>
>>  > + 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')
>
> And restore it here:
>
> serializer.cfg({encode_max_depth = orig_encode_max_depth})
Redone.

> BTW, I think test case with {encode_max_depth = 2} is redundant.
Removed.

>>  > + test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}',
>>  > + 'depth of encoding is 1 with .encode')
>>  > +
>
> Here we can check that json.encode(data, opts) did not change the global
> configuration:
>
> test:is(serializer.cfg.encode_max_depth, orig_encode_max_depth,
>     'global option remains unchanged')
Added.

> The same comments are applicable to other test cases.
Redone.

>>  > + 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')
>>  > +
>
> ecoding -> encoding (two occurences)
Fixed.

>>  > + test:ok(not pcall(serializer.decode, '{"1":{"b":{"c":1,"d":null}},"a":1}',
>>  > + {decode_max_depth = 2}),
>>  > + 'error: too many nested data structures')
>>  > +
>
> It is not obvious that gh-2888 block ends here. Please, add appropriate
> name for the block below or move gh-2888 block below the unnamed block.
Block is moved downwards.

> Look also how gh-3514 block header is formatted. It would be good to
> have all headers in the same style.
Done.

>>  > 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);
>
> lua_pop is used here (I guessto remove the table with options from a
> stack), but don't used in decode.
Added to decode.

>>  > + 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);
>
> encode_buf.length breaks incapsulation of strbuf 'object'. Please, use
> strbuf_length function instead.
Fixed.

commit 0d9351dac8312a6fab4a80c2167b75a36589d031
Author: Roman Khabibov <roman.habibov1@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.
    
    @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 2f0f4dc..653ed1c 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -215,6 +215,50 @@ static struct {
 };
 
 /**
+ * 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;
+	}
+	/*
+	 * 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);
+	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
  * luaL_serializer structure. serializer.cfg has overriden __call() method
@@ -225,39 +269,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 6b057af..f7980f4 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -240,6 +240,15 @@ luaL_checkserializer(struct lua_State *L) {
 		luaL_checkudata(L, lua_upvalueindex(1), LUAL_SERIALIZER);
 }
 
+/**
+ * Parse configuration table into @a cfg.
+ * @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 42c79d6..7ec3b11 100755
--- a/test/app-tap/json.test.lua
+++ b/test/app-tap/json.test.lua
@@ -22,7 +22,8 @@ end
 
 tap.test("json", function(test)
     local serializer = require('json')
-    test:plan(13)
+    test:plan(28)
+
     test:test("unsigned", common.test_unsigned, serializer)
     test:test("signed", common.test_signed, serializer)
     test:test("double", common.test_double, serializer)
@@ -34,6 +35,65 @@ tap.test("json", function(test)
     test:test("misc", test_misc, serializer)
 
     --
+    -- gh-2888: Check the possibility of using options in encode()/decode().
+    --
+    local orig_encode_max_depth = serializer.cfg.encode_max_depth
+    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 = orig_encode_max_depth})
+    test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}',
+            'depth of encoding is 1 with .encode')
+    test:is(serializer.cfg.encode_max_depth, orig_encode_max_depth,
+            'global option remains unchanged')
+
+    local orig_encode_invalid_numbers = serializer.cfg.encode_invalid_numbers
+    local nan = 1/0
+    serializer.cfg({encode_invalid_numbers = false})
+    test:ok(not pcall(serializer.encode, {a = nan}),
+            'expected error with NaN encoding with .cfg')
+    serializer.cfg({encode_invalid_numbers = orig_encode_invalid_numbers})
+    test:ok(not pcall(serializer.encode, {a = nan},
+                      {encode_invalid_numbers = false}),
+            'expected error with NaN encoding with .encode')
+    test:is(serializer.cfg.encode_invalid_numbers, orig_encode_invalid_numbers,
+            'global option remains unchanged')
+
+    local orig_encode_number_precision = serializer.cfg.encode_number_precision
+    local number = 0.12345
+    serializer.cfg({encode_number_precision = 3})
+    test:ok(serializer.encode({a = number}) == '{"a":0.123}',
+            'precision is 3')
+    serializer.cfg({encode_number_precision = orig_encode_number_precision})
+    test:ok(serializer.encode({a = number}, {encode_number_precision = 3}) ==
+            '{"a":0.123}', 'precision is 3')
+    test:is(serializer.cfg.encode_number_precision, orig_encode_number_precision,
+            'global option remains unchanged')
+
+    local orig_decode_invalid_numbers = serializer.cfg.decode_invalid_numbers
+    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 = orig_decode_invalid_numbers})
+    test:ok(not pcall(serializer.decode, '{"a":inf}',
+                      {decode_invalid_numbers = false}),
+            'expected error with NaN decoding with .decode')
+    test:is(serializer.cfg.decode_invalid_numbers, orig_decode_invalid_numbers,
+            'global option remains unchanged')
+
+    local orig_decode_max_depth = serializer.cfg.decode_max_depth
+    serializer.cfg({decode_max_depth = 2})
+    test:ok(not pcall(serializer.decode, '{"1":{"b":{"c":1,"d":null}},"a":1}'),
+            'error: too many nested data structures')
+    serializer.cfg({decode_max_depth = orig_decode_max_depth})
+    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:is(serializer.cfg.decode_max_depth, orig_decode_max_depth,
+            'global option remains unchanged')
+
+    --
     -- gh-3514: fix parsing integers with exponent in json
     --
     test:is(serializer.decode('{"var":2.0e+3}')["var"], 2000)
diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index 11aa402..631c53e 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;
-
-    luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
+static int json_encode(lua_State *l) {
+    luaL_argcheck(l, lua_gettop(l) == 2 || lua_gettop(l) == 1, 1,
+                  "expected 1 or 2 arguments");
 
-    /* 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, strbuf_length(&encode_buf));
     return 1;
 }
 
@@ -977,9 +980,18 @@ 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);
+        lua_pop(l, 1);
+        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;

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

[Alexander Turenko]

The patch looks good for me.

One minor comment is below.

Kirill, can you look into the patch? Two review iterations were passed.

branch: romankhabibov/gh-2888-options-encode
issue: https://github.com/tarantool/tarantool/issues/2888

WBR, Alexander Turenko.

On Mon, Sep 10, 2018 at 02:42:54AM +0300, roman.habibov1@yandex.ru wrote:
> Hi! Thanks for review.
> 
>      --
> +    -- gh-2888: Check the possibility of using options in encode()/decode().
> +    --
> +    local orig_encode_max_depth = serializer.cfg.encode_max_depth
> +    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 = orig_encode_max_depth})
> +    test:ok(serializer.encode(sub, {encode_max_depth = 1}) == '{"1":null,"a":1}',
> +            'depth of encoding is 1 with .encode')

test:ok(got == exp, msg) can be replaced with test:is(got, exp, msg) and
will show what was expected and what was got in case of fail.

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

[Vladislav Shpilevoy]

Also I've pushed more review fixes in a separate
commit. Please, pull it, look at and squash with
your one.

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

[Kirill Yukhin]

Hello,
On 08 июл 03:57, Roman Khabibov wrote:
> Added an ability to pass encoder options to json.encode().
> 
> Closes: #2888.
I've checked the patch into 1.9 branch.

--
Regards, Kirill Yukhin

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

[Vladislav Shpilevoy]

On 08/07/2018 03:57, Roman Khabibov wrote:
> Added an ability to pass encoder options to json.encode().
> 
> Closes: #2888.
> ---

As I asked you for the previous task, please, put here
branch and issue links. I know, that you use ./create_commits,
but it is not a standard instrument and has its own limitations.

For single-commit patches you should open the email in a text
editor and put here the links manually.

>   test/app-tap/json.test.lua        | 27 +++++++++++-
>   third_party/lua-cjson/lua_cjson.c | 90 +++++++++++++++++++++++++++++++++------
>   2 files changed, 104 insertions(+), 13 deletions(-)
> 

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

[Kirill Yukhin]

Hello,
On 08 июл 03:57, Roman Khabibov wrote:
> Added an ability to pass encoder options to json.encode().
> 
> Closes: #2888.

I've checked your updated patch into 1.10 branch.

--
Regards, Kirill Yukhin