[tarantool-patches] [PATCH 5/7] msgpack: allow to pass 'struct ibuf *' into encode()

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Before the patch msgpack Lua module provided a method encode()
able to take a custom buffer to encode into. But it should be of
type 'struct ibuf', what made it impossible to use
buffer.IBUF_SHARED as a buffer, because its type is
'struct ibuf *'. Strangely, but FFI can't convert these types
automatically.

This commit allows to use 'struct ibuf *' as well, and moves this
functionality into a function in utils.h. Now both msgpack and
merger modules can use ibuf directly and by pointer.
---
 src/box/lua/merger.c      | 24 ++----------------------
 src/lua/msgpack.c         | 12 +++---------
 src/lua/utils.c           | 25 +++++++++++++++++++++++++
 src/lua/utils.h           |  8 ++++++++
 test/app/msgpack.result   | 26 +++++++++++++++++++++++++-
 test/app/msgpack.test.lua |  9 +++++++++
 6 files changed, 72 insertions(+), 32 deletions(-)

diff --git a/src/box/lua/merger.c b/src/box/lua/merger.c
index a61adb389..1b155152b 100644
--- a/src/box/lua/merger.c
+++ b/src/box/lua/merger.c
@@ -59,7 +59,6 @@
 #include "box/merger.h"      /* merge_source_*, merger_*() */
 
 static uint32_t CTID_STRUCT_MERGE_SOURCE_REF = 0;
-static uint32_t CTID_STRUCT_IBUF = 0;
 
 /**
  * A type of a function to create a source from a Lua iterator on
@@ -72,22 +71,6 @@ typedef struct merge_source *(*luaL_merge_source_new_f)(struct lua_State *L);
 
 /* {{{ Helpers */
 
-/**
- * Extract an ibuf object from the Lua stack.
- */
-static struct ibuf *
-luaT_check_ibuf(struct lua_State *L, int idx)
-{
-	if (lua_type(L, idx) != LUA_TCDATA)
-		return NULL;
-
-	uint32_t cdata_type;
-	struct ibuf *ibuf_ptr = luaL_checkcdata(L, idx, &cdata_type);
-	if (ibuf_ptr == NULL || cdata_type != CTID_STRUCT_IBUF)
-		return NULL;
-	return ibuf_ptr;
-}
-
 /**
  * Extract a merge source from the Lua stack.
  */
@@ -446,7 +429,7 @@ luaL_merge_source_buffer_fetch(struct merge_source_buffer *source)
 		source->ref = 0;
 	}
 	lua_pushvalue(L, -nresult + 1); /* Popped by luaL_ref(). */
-	source->buf = luaT_check_ibuf(L, -1);
+	source->buf = luaL_checkibuf(L, -1);
 	if (source->buf == NULL) {
 		diag_set(IllegalParams, "Expected <state>, <buffer>");
 		return -1;
@@ -1082,7 +1065,7 @@ lbox_merge_source_select(struct lua_State *L)
 		lua_pushstring(L, "buffer");
 		lua_gettable(L, 2);
 		if (!lua_isnil(L, -1)) {
-			if ((output_buffer = luaT_check_ibuf(L, -1)) == NULL)
+			if ((output_buffer = luaL_checkibuf(L, -1)) == NULL)
 				return lbox_merge_source_select_usage(L,
 					"buffer");
 		}
@@ -1116,10 +1099,7 @@ LUA_API int
 luaopen_merger(struct lua_State *L)
 {
 	luaL_cdef(L, "struct merge_source;");
-	luaL_cdef(L, "struct ibuf;");
-
 	CTID_STRUCT_MERGE_SOURCE_REF = luaL_ctypeid(L, "struct merge_source&");
-	CTID_STRUCT_IBUF = luaL_ctypeid(L, "struct ibuf");
 
 	/* Export C functions to Lua. */
 	static const struct luaL_Reg meta[] = {
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index d3cfc0e2b..73bda80ea 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -51,7 +51,6 @@ luamp_error(void *error_ctx)
 }
 
 static uint32_t CTID_CHAR_PTR;
-static uint32_t CTID_STRUCT_IBUF;
 
 struct luaL_serializer *luaL_msgpack_default = NULL;
 
@@ -301,11 +300,11 @@ lua_msgpack_encode(lua_State *L)
 
 	struct ibuf *buf;
 	if (index > 1) {
-		uint32_t ctypeid;
-		buf = luaL_checkcdata(L, 2, &ctypeid);
-		if (ctypeid != CTID_STRUCT_IBUF)
+		buf = luaL_checkibuf(L, 2);
+		if (buf == NULL) {
 			return luaL_error(L, "msgpack.encode: argument 2 "
 					  "must be of type 'struct ibuf'");
+		}
 	} else {
 		buf = tarantool_lua_ibuf;
 		ibuf_reset(buf);
@@ -526,11 +525,6 @@ lua_msgpack_new(lua_State *L)
 LUALIB_API int
 luaopen_msgpack(lua_State *L)
 {
-	int rc = luaL_cdef(L, "struct ibuf;");
-	assert(rc == 0);
-	(void) rc;
-	CTID_STRUCT_IBUF = luaL_ctypeid(L, "struct ibuf");
-	assert(CTID_STRUCT_IBUF != 0);
 	CTID_CHAR_PTR = luaL_ctypeid(L, "char *");
 	assert(CTID_CHAR_PTR != 0);
 	luaL_msgpack_default = luaL_newserializer(L, "msgpack", msgpacklib);
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 192912ab8..f53d7e588 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -41,6 +41,9 @@ int luaL_nil_ref = LUA_REFNIL;
 int luaL_map_metatable_ref = LUA_REFNIL;
 int luaL_array_metatable_ref = LUA_REFNIL;
 
+static uint32_t CTID_STRUCT_IBUF;
+static uint32_t CTID_STRUCT_IBUF_PTR;
+
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
 {
@@ -1059,6 +1062,20 @@ luaL_iscallable(lua_State *L, int idx)
 	return res;
 }
 
+struct ibuf *
+luaL_checkibuf(struct lua_State *L, int idx)
+{
+	if (lua_type(L, idx) != LUA_TCDATA)
+		return NULL;
+	uint32_t cdata_type;
+	void *cdata = luaL_checkcdata(L, idx, &cdata_type);
+	if (cdata_type == CTID_STRUCT_IBUF)
+		return (struct ibuf *) cdata;
+	if (cdata_type == CTID_STRUCT_IBUF_PTR && cdata != NULL)
+		return *(struct ibuf **) cdata;
+	return NULL;
+}
+
 lua_State *
 luaT_state(void)
 {
@@ -1185,5 +1202,13 @@ tarantool_lua_utils_init(struct lua_State *L)
 	lua_setfield(L, -2, "__newindex");
 	luaL_array_metatable_ref = luaL_ref(L, LUA_REGISTRYINDEX);
 
+	int rc = luaL_cdef(L, "struct ibuf;");
+	assert(rc == 0);
+	(void) rc;
+	CTID_STRUCT_IBUF = luaL_ctypeid(L, "struct ibuf");
+	assert(CTID_STRUCT_IBUF != 0);
+	CTID_STRUCT_IBUF_PTR = luaL_ctypeid(L, "struct ibuf *");
+	assert(CTID_STRUCT_IBUF_PTR != 0);
+
 	return 0;
 }
diff --git a/src/lua/utils.h b/src/lua/utils.h
index a22492227..cf51eb132 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -536,6 +536,14 @@ luaL_checkfinite(struct lua_State *L, struct luaL_serializer *cfg,
 		luaL_error(L, "number must not be NaN or Inf");
 }
 
+/**
+ * Check if a value on @a L stack by index @a idx is an ibuf
+ * object. Both 'struct ibuf' and 'struct ibuf *' are accepted.
+ * Returns NULL, if can't convert - not an ibuf object.
+ */
+struct ibuf *
+luaL_checkibuf(struct lua_State *L, int idx);
+
 /* {{{ Helper functions to interact with a Lua iterator from C */
 
 /**
diff --git a/test/app/msgpack.result b/test/app/msgpack.result
index b1fe4e53b..9fc42fc3c 100644
--- a/test/app/msgpack.result
+++ b/test/app/msgpack.result
@@ -14,7 +14,7 @@ msgpack.encode()
 ...
 msgpack.encode('test', 'str')
 ---
-- error: expected cdata as 2 argument
+- error: 'msgpack.encode: argument 2 must be of type ''struct ibuf'''
 ...
 msgpack.encode('test', buf.buf)
 ---
@@ -202,3 +202,27 @@ msgpack.decode(buf.rpos, buf:size() - 1)
 ---
 - error: 'msgpack.decode: invalid MsgPack'
 ...
+-- Provide a buffer. Try both 'struct ibuf' and 'struct ibuf *'.
+buf = buffer.IBUF_SHARED
+---
+...
+buf:reset()
+---
+...
+size = msgpack.encode({a = 1, b = 2}, buf)
+---
+...
+(msgpack.decode(buf.rpos, size))
+---
+- {'a': 1, 'b': 2}
+...
+buf = buffer.ibuf()
+---
+...
+size = msgpack.encode({c = 3, d = 4}, buf)
+---
+...
+(msgpack.decode(buf.rpos, size))
+---
+- {'c': 3, 'd': 4}
+...
diff --git a/test/app/msgpack.test.lua b/test/app/msgpack.test.lua
index 09c3dec5d..0920fa507 100644
--- a/test/app/msgpack.test.lua
+++ b/test/app/msgpack.test.lua
@@ -62,3 +62,12 @@ msgpack.decode(s)
 buf = buffer.ibuf()
 msgpack.encode({1, 2, 3}, buf)
 msgpack.decode(buf.rpos, buf:size() - 1)
+
+-- Provide a buffer. Try both 'struct ibuf' and 'struct ibuf *'.
+buf = buffer.IBUF_SHARED
+buf:reset()
+size = msgpack.encode({a = 1, b = 2}, buf)
+(msgpack.decode(buf.rpos, size))
+buf = buffer.ibuf()
+size = msgpack.encode({c = 3, d = 4}, buf)
+(msgpack.decode(buf.rpos, size))
-- 
2.20.1 (Apple Git-117)