[tarantool-patches] Re: [PATCH] json: added options to json.encode()
- From: roman.habibov1@xxxxxxxxx
- To: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>, "tarantool-patches@xxxxxxxxxxxxx" <tarantool-patches@xxxxxxxxxxxxx>
- Date: Tue, 17 Jul 2018 21:19:46 +0300
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;
Other related posts:
