[tarantool-patches] [PATCH 0/7] swim lua preparation, again

[tarantool-patches] [PATCH 0/7] swim lua preparation, again

[Vladislav Shpilevoy]

This patchset consists of patches quite independent and needed for proper
functioning of SWIM Lua API. Most of them are refactoring, and a couple of bug
fixes.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/swim-lua-preparation
Issue: https://github.com/tarantool/tarantool/issues/3234

Vladislav Shpilevoy (7):
  swim: drop swim_info() function
  swim: encapsulate 'uint16' payload size inside swim.c
  swim: do not rebind when new 'port' is 0
  swim: set 'left' status in self on swim_quit()
  msgpack: allow to pass 'struct ibuf *' into encode()
  msgpack: allow to pass 'const char *' into decode()
  Drop an unused function and class

 src/box/lua/merger.c              | 24 ++---------------
 src/lib/swim/swim.c               | 29 ++++----------------
 src/lib/swim/swim.h               |  9 ++-----
 src/lib/swim/swim_transport_udp.c | 14 ++++++++--
 src/lua/msgpack.c                 | 28 ++++++--------------
 src/lua/utils.c                   | 43 ++++++++++++++++++++++++++++++
 src/lua/utils.h                   | 44 +++++++++++++------------------
 test/app-tap/msgpack.test.lua     |  7 ++---
 test/app/msgpack.result           | 44 ++++++++++++++++++++++++++++++-
 test/app/msgpack.test.lua         | 16 +++++++++++
 test/unit/swim.c                  | 20 +++++++++-----
 test/unit/swim.result             | 21 ++++++++-------
 test/unit/swim_test_utils.c       | 12 ++++-----
 test/unit/swim_test_utils.h       |  6 ++---
 14 files changed, 187 insertions(+), 130 deletions(-)

-- 
2.20.1 (Apple Git-117)

[tarantool-patches] [PATCH 1/7] swim: drop swim_info() function

[Vladislav Shpilevoy]

Swim_info() was a function to dump SWIM instance info to a Lua
table without explicit usage of Lua. But now all the info can be
taken from 1) self member and member API, 2) cached cfg options
as a Lua table in a forthcoming Lua API - this is how
box.cfg.<index> works.
---
 src/lib/swim/swim.c | 20 --------------------
 src/lib/swim/swim.h |  5 -----
 2 files changed, 25 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index fa6b3a379..006c265b3 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -35,7 +35,6 @@
 #include "uri/uri.h"
 #include "fiber.h"
 #include "msgpuck.h"
-#include "info/info.h"
 #include "assoc.h"
 #include "sio.h"
 #define HEAP_FORWARD_DECLARATION
@@ -1911,25 +1910,6 @@ swim_broadcast(struct swim *swim, int port)
 	return 0;
 }
 
-void
-swim_info(struct swim *swim, struct info_handler *info)
-{
-	info_begin(info);
-	for (mh_int_t node = mh_first(swim->members),
-	     end = mh_end(swim->members); node != end;
-	     node = mh_next(swim->members, node)) {
-		struct swim_member *m =
-			*mh_swim_table_node(swim->members, node);
-		info_table_begin(info, swim_inaddr_str(&m->addr));
-		info_append_str(info, "status",
-				swim_member_status_strs[m->status]);
-		info_append_str(info, "uuid", tt_uuid_str(&m->uuid));
-		info_append_int(info, "incarnation", (int64_t) m->incarnation);
-		info_table_end(info);
-	}
-	info_end(info);
-}
-
 int
 swim_size(const struct swim *swim)
 {
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 331bd14f0..044558dc8 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -38,7 +38,6 @@
 extern "C" {
 #endif
 
-struct info_handler;
 struct swim;
 struct tt_uuid;
 struct swim_iterator;
@@ -153,10 +152,6 @@ swim_probe_member(struct swim *swim, const char *uri);
 int
 swim_broadcast(struct swim *swim, int port);
 
-/** Dump member statuses into @a info. */
-void
-swim_info(struct swim *swim, struct info_handler *info);
-
 /** Get SWIM member table size. */
 int
 swim_size(const struct swim *swim);
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/7] swim: drop swim_info() function

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> Swim_info() was a function to dump SWIM instance info to a Lua
> table without explicit usage of Lua. But now all the info can be
> taken from 1) self member and member API, 2) cached cfg options
> as a Lua table in a forthcoming Lua API - this is how
> box.cfg.<index> works.

ok to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 2/7] swim: encapsulate 'uint16' payload size inside swim.c

[Vladislav Shpilevoy]

uint16 was used in public SWIM C API as a type for payload size
to emphasize its small value. But it is not useful in Lua,
because Lua API should explicitly check if a number overflows
uint16 maximal value, and return the same error as in case it is
< uint16_max, but > payload_size_max.

So main motivation of the patch is to avoid unnecessary checks in
Lua and error message duplication. Internally payload size is
still uint16.
---
 src/lib/swim/swim.c         |  8 ++++----
 src/lib/swim/swim.h         |  4 ++--
 test/unit/swim.c            | 10 +++++-----
 test/unit/swim_test_utils.c | 12 ++++++------
 test/unit/swim_test_utils.h |  6 +++---
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 006c265b3..7a43fae1b 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -1824,10 +1824,10 @@ swim_is_configured(const struct swim *swim)
 }
 
 int
-swim_set_payload(struct swim *swim, const char *payload, uint16_t payload_size)
+swim_set_payload(struct swim *swim, const char *payload, int payload_size)
 {
-	if (payload_size > MAX_PAYLOAD_SIZE) {
-		diag_set(IllegalParams, "Payload should be <= %d",
+	if (payload_size > MAX_PAYLOAD_SIZE || payload_size < 0) {
+		diag_set(IllegalParams, "Payload should be <= %d and >= 0",
 			 MAX_PAYLOAD_SIZE);
 		return -1;
 	}
@@ -2070,7 +2070,7 @@ swim_member_incarnation(const struct swim_member *member)
 }
 
 const char *
-swim_member_payload(const struct swim_member *member, uint16_t *size)
+swim_member_payload(const struct swim_member *member, int *size)
 {
 	*size = member->payload_size;
 	return member->payload;
diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h
index 044558dc8..8c4f8c3cf 100644
--- a/src/lib/swim/swim.h
+++ b/src/lib/swim/swim.h
@@ -102,7 +102,7 @@ swim_cfg(struct swim *swim, const char *uri, double heartbeat_rate,
 
 /** Set payload to disseminate over the cluster. */
 int
-swim_set_payload(struct swim *swim, const char *payload, uint16_t payload_size);
+swim_set_payload(struct swim *swim, const char *payload, int payload_size);
 
 /**
  * Stop listening and broadcasting messages, cleanup all internal
@@ -218,7 +218,7 @@ swim_member_incarnation(const struct swim_member *member);
 
 /** Member's payload. */
 const char *
-swim_member_payload(const struct swim_member *member, uint16_t *size);
+swim_member_payload(const struct swim_member *member, int *size);
 
 /**
  * Reference a member. The member memory will be valid until unref
diff --git a/test/unit/swim.c b/test/unit/swim.c
index 8b4a1f17a..3ec25c5d0 100644
--- a/test/unit/swim.c
+++ b/test/unit/swim.c
@@ -672,7 +672,7 @@ static void
 swim_test_payload_basic(void)
 {
 	swim_start_test(11);
-	uint16_t size, cluster_size = 3;
+	int size, cluster_size = 3;
 	struct swim_cluster *cluster = swim_cluster_new(cluster_size);
 	for (int i = 0; i < cluster_size; ++i) {
 		for (int j = i + 1; j < cluster_size; ++j)
@@ -685,7 +685,7 @@ swim_test_payload_basic(void)
 	ok(swim_error_check_match("Payload should be <="), "diag says too big");
 
 	const char *s0_payload = "S1 payload";
-	uint16_t s0_payload_size = strlen(s0_payload) + 1;
+	int s0_payload_size = strlen(s0_payload) + 1;
 	is(swim_cluster_member_set_payload(cluster, 0, s0_payload,
 					   s0_payload_size), 0,
 	   "payload is set");
@@ -732,7 +732,7 @@ static void
 swim_test_payload_refutation(void)
 {
 	swim_start_test(11);
-	uint16_t size, cluster_size = 3;
+	int size, cluster_size = 3;
 	struct swim_cluster *cluster = swim_cluster_new(cluster_size);
 	swim_cluster_set_ack_timeout(cluster, 1);
 	for (int i = 0; i < cluster_size; ++i) {
@@ -740,7 +740,7 @@ swim_test_payload_refutation(void)
 			swim_cluster_interconnect(cluster, i, j);
 	}
 	const char *s0_old_payload = "s0 payload";
-	uint16_t s0_old_payload_size = strlen(s0_old_payload) + 1;
+	int s0_old_payload_size = strlen(s0_old_payload) + 1;
 	fail_if(swim_cluster_member_set_payload(cluster, 0, s0_old_payload,
 						s0_old_payload_size) != 0);
 	fail_if(swim_cluster_wait_payload_everywhere(cluster, 0, s0_old_payload,
@@ -757,7 +757,7 @@ swim_test_payload_refutation(void)
 	 * lost, however ACKs work ok.
 	 */
 	const char *s0_new_payload = "s0 second payload";
-	uint16_t s0_new_payload_size = strlen(s0_new_payload);
+	int s0_new_payload_size = strlen(s0_new_payload);
 	fail_if(swim_cluster_member_set_payload(cluster, 0, s0_new_payload,
 						s0_new_payload_size) != 0);
 	int components[2] = {SWIM_DISSEMINATION, SWIM_ANTI_ENTROPY};
diff --git a/test/unit/swim_test_utils.c b/test/unit/swim_test_utils.c
index de1bef6e7..f55388055 100644
--- a/test/unit/swim_test_utils.c
+++ b/test/unit/swim_test_utils.c
@@ -332,7 +332,7 @@ swim_cluster_member_incarnation(struct swim_cluster *cluster, int node_id,
 
 const char *
 swim_cluster_member_payload(struct swim_cluster *cluster, int node_id,
-			    int member_id, uint16_t *size)
+			    int member_id, int *size)
 {
 	const struct swim_member *m =
 		swim_cluster_member_view(cluster, node_id, member_id);
@@ -345,7 +345,7 @@ swim_cluster_member_payload(struct swim_cluster *cluster, int node_id,
 
 int
 swim_cluster_member_set_payload(struct swim_cluster *cluster, int i,
-				const char *payload, uint16_t size)
+				const char *payload, int size)
 {
 	struct swim *s = swim_cluster_member(cluster, i);
 	return swim_set_payload(s, payload, size);
@@ -687,7 +687,7 @@ struct swim_member_template {
 	 */
 	bool need_check_payload;
 	const char *payload;
-	uint16_t payload_size;
+	int payload_size;
 };
 
 /** Build member template. No checks are set. */
@@ -730,7 +730,7 @@ swim_member_template_set_incarnation(struct swim_member_template *t,
  */
 static inline void
 swim_member_template_set_payload(struct swim_member_template *t,
-				 const char *payload, uint16_t payload_size)
+				 const char *payload, int payload_size)
 {
 	t->need_check_payload = true;
 	t->payload = payload;
@@ -747,7 +747,7 @@ swim_loop_check_member(struct swim_cluster *cluster, void *data)
 	enum swim_member_status status;
 	uint64_t incarnation;
 	const char *payload;
-	uint16_t payload_size;
+	int payload_size;
 	if (m != NULL) {
 		status = swim_member_status(m);
 		incarnation = swim_member_incarnation(m);
@@ -851,7 +851,7 @@ swim_cluster_wait_status_everywhere(struct swim_cluster *cluster, int member_id,
 int
 swim_cluster_wait_payload_everywhere(struct swim_cluster *cluster,
 				     int member_id, const char *payload,
-				     uint16_t payload_size, double timeout)
+				     int payload_size, double timeout)
 {
 	struct swim_member_template t;
 	swim_member_template_create(&t, -1, member_id);
diff --git a/test/unit/swim_test_utils.h b/test/unit/swim_test_utils.h
index c78894820..b786dfd79 100644
--- a/test/unit/swim_test_utils.h
+++ b/test/unit/swim_test_utils.h
@@ -154,11 +154,11 @@ swim_cluster_member_incarnation(struct swim_cluster *cluster, int node_id,
 
 const char *
 swim_cluster_member_payload(struct swim_cluster *cluster, int node_id,
-			    int member_id, uint16_t *size);
+			    int member_id, int *size);
 
 int
 swim_cluster_member_set_payload(struct swim_cluster *cluster, int i,
-				const char *payload, uint16_t size);
+				const char *payload, int size);
 
 /**
  * Check if in the cluster every instance knowns the about other
@@ -219,7 +219,7 @@ swim_cluster_wait_incarnation(struct swim_cluster *cluster, int node_id,
 int
 swim_cluster_wait_payload_everywhere(struct swim_cluster *cluster,
 				     int member_id, const char *payload,
-				     uint16_t payload_size, double timeout);
+				     int payload_size, double timeout);
 
 /** Process SWIM events for @a duration fake seconds. */
 void
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 2/7] swim: encapsulate 'uint16' payload size inside swim.c

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> uint16 was used in public SWIM C API as a type for payload size
> to emphasize its small value. But it is not useful in Lua,
> because Lua API should explicitly check if a number overflows
> uint16 maximal value, and return the same error as in case it is
> < uint16_max, but > payload_size_max.

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 3/7] swim: do not rebind when new 'port' is 0

[Vladislav Shpilevoy]

SWIM internally tries to avoid unnecessary close+socket+bind
calls on reconfiguration if a new URI is the same as an old one.
SWIM transport compares <IP, port> couples and if they are
equal, does nothing.

But if a port is 0, it is not a real port, but a sign to the
kernel to find any free port on the IP address. In such a case
SWIM transport after bind() retrieves and saves a real port.

When the same URI is specified again, the transport compares two
addresses: old <IP, auto found port>, new <IP, 0>, sees they are
'different', and rebinds. It is not necessary, obviously, because
the new URI covers the old one.

This commit avoids rebind, when new IP == old IP, and new port is
0.

Part of #3234
---
 src/lib/swim/swim_transport_udp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/lib/swim/swim_transport_udp.c b/src/lib/swim/swim_transport_udp.c
index f017eeaf0..c0317a20b 100644
--- a/src/lib/swim/swim_transport_udp.c
+++ b/src/lib/swim/swim_transport_udp.c
@@ -64,11 +64,21 @@ swim_transport_bind(struct swim_transport *transport,
 	const struct sockaddr_in *new_addr = (const struct sockaddr_in *) addr;
 	const struct sockaddr_in *old_addr = &transport->addr;
 	assert(addr_len == sizeof(*new_addr));
+	bool is_new_port_any = new_addr->sin_port == 0;
 
 	if (transport->fd != -1 &&
 	    new_addr->sin_addr.s_addr == old_addr->sin_addr.s_addr &&
-	    new_addr->sin_port == old_addr->sin_port)
+	    (new_addr->sin_port == old_addr->sin_port || is_new_port_any)) {
+		/*
+		 * Note, that new port == 0 means that any port is
+		 * ok. If at the same time old and new IP
+		 * addresses are the same and the socket is
+		 * already bound (fd != -1), then the existing
+		 * socket 'matches' the new URI and rebind is not
+		 * needed.
+		 */
 		return 0;
+	}
 
 	int fd = sio_socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
 	if (fd < 0)
@@ -81,7 +91,7 @@ swim_transport_bind(struct swim_transport *transport,
 		return -1;
 	}
 	int real_port = new_addr->sin_port;
-	if (new_addr->sin_port == 0) {
+	if (is_new_port_any) {
 		struct sockaddr_in real_addr;
 		addr_len = sizeof(real_addr);
 		if (sio_getsockname(fd, (struct sockaddr *) &real_addr,
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 3/7] swim: do not rebind when new 'port' is 0

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> SWIM internally tries to avoid unnecessary close+socket+bind
> calls on reconfiguration if a new URI is the same as an old one.
> SWIM transport compares <IP, port> couples and if they are
> equal, does nothing.
> 
> But if a port is 0, it is not a real port, but a sign to the
> kernel to find any free port on the IP address. In such a case
> SWIM transport after bind() retrieves and saves a real port.
> 
> When the same URI is specified again, the transport compares two
> addresses: old <IP, auto found port>, new <IP, 0>, sees they are
> 'different', and rebinds. It is not necessary, obviously, because
> the new URI covers the old one.
> 
> This commit avoids rebind, when new IP == old IP, and new port is
> 0.

ok to push


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 4/7] swim: set 'left' status in self on swim_quit()

[Vladislav Shpilevoy]

swim_quit() notifies all the members that this instance has left
the cluster. Strangely, except self. It is not a real bug, but
showing 'left' status in self struct swim_member would be more
correct than 'alive', obviously.

It is possible, that self struct swim_member was referenced by a
user - this is how 'self' can be available after SWIM instance
deletion.

Part of #3234
---
 src/lib/swim/swim.c   |  1 +
 test/unit/swim.c      | 10 ++++++++--
 test/unit/swim.result | 21 +++++++++++----------
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 7a43fae1b..54c5b3250 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -2002,6 +2002,7 @@ swim_quit(struct swim *swim)
 	assert(rc == 2);
 	mp_encode_map(header, rc);
 	swim_quit_step_complete(task, &swim->scheduler, 0);
+	swim->self->status = MEMBER_LEFT;
 }
 
 struct swim_member *
diff --git a/test/unit/swim.c b/test/unit/swim.c
index 3ec25c5d0..d9613e8e0 100644
--- a/test/unit/swim.c
+++ b/test/unit/swim.c
@@ -502,14 +502,20 @@ swim_test_undead(void)
 static void
 swim_test_quit(void)
 {
-	swim_start_test(9);
+	swim_start_test(10);
 	int size = 3;
 	struct swim_cluster *cluster = swim_cluster_new(size);
 	for (int i = 0; i < size; ++i) {
 		for (int j = 0; j < size; ++j)
 			swim_cluster_add_link(cluster, i, j);
 	}
+	struct swim *s0 = swim_cluster_member(cluster, 0);
+	struct swim_member *s0_self = swim_self(s0);
+	swim_member_ref(s0_self);
 	swim_cluster_quit_node(cluster, 0);
+	is(swim_member_status(s0_self), MEMBER_LEFT,
+	   "'self' is 'left' immediately after quit");
+	swim_member_unref(s0_self);
 	is(swim_cluster_wait_status_everywhere(cluster, 0, MEMBER_LEFT, 0),
 	   0, "'quit' is sent to all the members without delays between "\
 	   "dispatches")
@@ -528,7 +534,7 @@ swim_test_quit(void)
 	 * received the 'quit' message with the same UUID. Of
 	 * course, it should be refuted.
 	 */
-	struct swim *s0 = swim_cluster_member(cluster, 0);
+	s0 = swim_cluster_member(cluster, 0);
 	struct tt_uuid s0_uuid = *swim_member_uuid(swim_self(s0));
 	struct swim *s1 = swim_cluster_member(cluster, 1);
 	swim_remove_member(s1, &s0_uuid);
diff --git a/test/unit/swim.result b/test/unit/swim.result
index 0d1fa4a32..266d83589 100644
--- a/test/unit/swim.result
+++ b/test/unit/swim.result
@@ -122,16 +122,17 @@ ok 11 - subtests
 ok 12 - subtests
 	*** swim_test_packet_loss: done ***
 	*** swim_test_quit ***
-    1..9
-    ok 1 - 'quit' is sent to all the members without delays between dispatches
-    ok 2 - quited member S1 has returned and refuted the old status
-    ok 3 - another member S2 has taken the quited UUID
-    ok 4 - S3 did not add S1 back when received its 'quit'
-    ok 5 - S2 finally got 'quit' message from S1, but with its 'own' UUID - refute it
-    ok 6 - S3 sees S1 as left
-    ok 7 - S2 does not see S1 at all
-    ok 8 - after more time S1 is dropped from S3
-    ok 9 - and still is not added to S2 - left members can not be added
+    1..10
+    ok 1 - 'self' is 'left' immediately after quit
+    ok 2 - 'quit' is sent to all the members without delays between dispatches
+    ok 3 - quited member S1 has returned and refuted the old status
+    ok 4 - another member S2 has taken the quited UUID
+    ok 5 - S3 did not add S1 back when received its 'quit'
+    ok 6 - S2 finally got 'quit' message from S1, but with its 'own' UUID - refute it
+    ok 7 - S3 sees S1 as left
+    ok 8 - S2 does not see S1 at all
+    ok 9 - after more time S1 is dropped from S3
+    ok 10 - and still is not added to S2 - left members can not be added
 ok 13 - subtests
 	*** swim_test_quit: done ***
 	*** swim_test_uri_update ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 4/7] swim: set 'left' status in self on swim_quit()

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> swim_quit() notifies all the members that this instance has left
> the cluster. Strangely, except self. It is not a real bug, but
> showing 'left' status in self struct swim_member would be more
> correct than 'alive', obviously.
> 
> It is possible, that self struct swim_member was referenced by a
> user - this is how 'self' can be available after SWIM instance
> deletion.

ok to push


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

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

[Vladislav Shpilevoy]

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)

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

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> 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.

ok to push


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 6/7] msgpack: allow to pass 'const char *' into decode()

[Vladislav Shpilevoy]

msgpack.decode() internally uses 'const char *' variable to
decode msgpack, but somewhy expects only 'char *' as input.
This commit allows to pass 'const char *' as well.
---
 src/lua/msgpack.c             | 16 +++++-----------
 src/lua/utils.c               | 20 +++++++++++++++++++-
 src/lua/utils.h               | 11 +++++++++++
 test/app-tap/msgpack.test.lua |  7 ++++---
 test/app/msgpack.result       | 18 ++++++++++++++++++
 test/app/msgpack.test.lua     |  7 +++++++
 6 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index 73bda80ea..66b83b894 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -50,8 +50,6 @@ luamp_error(void *error_ctx)
 	luaL_error(L, diag_last_error(diag_get())->errmsg);
 }
 
-static uint32_t CTID_CHAR_PTR;
-
 struct luaL_serializer *luaL_msgpack_default = NULL;
 
 static enum mp_type
@@ -332,9 +330,8 @@ lua_msgpack_encode(lua_State *L)
 static int
 lua_msgpack_decode_cdata(lua_State *L, bool check)
 {
-	uint32_t ctypeid;
-	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
-	if (ctypeid != CTID_CHAR_PTR) {
+	const char *data;
+	if (luaL_checkconstchar(L, 1, &data) != 0) {
 		return luaL_error(L, "msgpack.decode: "
 				  "a Lua string or 'char *' expected");
 	}
@@ -346,7 +343,7 @@ lua_msgpack_decode_cdata(lua_State *L, bool check)
 	}
 	struct luaL_serializer *cfg = luaL_checkserializer(L);
 	luamp_decode(L, cfg, &data);
-	*(const char **)luaL_pushcdata(L, ctypeid) = data;
+	*(const char **)luaL_pushcdata(L, CTID_CHAR_PTR) = data;
 	return 2;
 }
 
@@ -435,9 +432,8 @@ verify_decode_header_args(lua_State *L, const char *func_name,
 		return luaL_error(L, "Usage: %s(ptr, size)", func_name);
 
 	/* Verify ptr type. */
-	uint32_t ctypeid;
-	const char *data = *(char **) luaL_checkcdata(L, 1, &ctypeid);
-	if (ctypeid != CTID_CHAR_PTR)
+	const char *data;
+	if (luaL_checkconstchar(L, 1, &data) != 0)
 		return luaL_error(L, "%s: 'char *' expected", func_name);
 
 	/* Verify size type and value. */
@@ -525,8 +521,6 @@ lua_msgpack_new(lua_State *L)
 LUALIB_API int
 luaopen_msgpack(lua_State *L)
 {
-	CTID_CHAR_PTR = luaL_ctypeid(L, "char *");
-	assert(CTID_CHAR_PTR != 0);
 	luaL_msgpack_default = luaL_newserializer(L, "msgpack", msgpacklib);
 	return 1;
 }
diff --git a/src/lua/utils.c b/src/lua/utils.c
index f53d7e588..01a0cd894 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -43,6 +43,8 @@ int luaL_array_metatable_ref = LUA_REFNIL;
 
 static uint32_t CTID_STRUCT_IBUF;
 static uint32_t CTID_STRUCT_IBUF_PTR;
+uint32_t CTID_CHAR_PTR;
+uint32_t CTID_CONST_CHAR_PTR;
 
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
@@ -1076,6 +1078,19 @@ luaL_checkibuf(struct lua_State *L, int idx)
 	return NULL;
 }
 
+int
+luaL_checkconstchar(struct lua_State *L, int idx, const char **res)
+{
+	if (lua_type(L, idx) != LUA_TCDATA)
+		return -1;
+	uint32_t cdata_type;
+	void *cdata = luaL_checkcdata(L, idx, &cdata_type);
+	if (cdata_type != CTID_CHAR_PTR && cdata_type != CTID_CONST_CHAR_PTR)
+		return -1;
+	*res = cdata != NULL ? *(const char **) cdata : NULL;
+	return 0;
+}
+
 lua_State *
 luaT_state(void)
 {
@@ -1209,6 +1224,9 @@ tarantool_lua_utils_init(struct lua_State *L)
 	assert(CTID_STRUCT_IBUF != 0);
 	CTID_STRUCT_IBUF_PTR = luaL_ctypeid(L, "struct ibuf *");
 	assert(CTID_STRUCT_IBUF_PTR != 0);
-
+	CTID_CHAR_PTR = luaL_ctypeid(L, "char *");
+	assert(CTID_CHAR_PTR != 0);
+	CTID_CONST_CHAR_PTR = luaL_ctypeid(L, "const char *");
+	assert(CTID_CONST_CHAR_PTR != 0);
 	return 0;
 }
diff --git a/src/lua/utils.h b/src/lua/utils.h
index cf51eb132..3d887a5ce 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -67,6 +67,9 @@ struct ibuf;
 extern struct lua_State *tarantool_L;
 extern struct ibuf *tarantool_lua_ibuf;
 
+extern uint32_t CTID_CONST_CHAR_PTR;
+extern uint32_t CTID_CHAR_PTR;
+
 /** \cond public */
 
 /**
@@ -544,6 +547,14 @@ luaL_checkfinite(struct lua_State *L, struct luaL_serializer *cfg,
 struct ibuf *
 luaL_checkibuf(struct lua_State *L, int idx);
 
+/**
+ * Check if a value on @a L stack by index @a idx is pointer at
+ * char or const char. '(char *)NULL' is also considered a valid
+ * char pointer.
+ */
+int
+luaL_checkconstchar(struct lua_State *L, int idx, const char **res);
+
 /* {{{ Helper functions to interact with a Lua iterator from C */
 
 /**
diff --git a/test/app-tap/msgpack.test.lua b/test/app-tap/msgpack.test.lua
index 1b0bb9806..bd095e5ae 100755
--- a/test/app-tap/msgpack.test.lua
+++ b/test/app-tap/msgpack.test.lua
@@ -57,6 +57,7 @@ local function test_decode_array_map_header(test, s)
         'of buffer'
     local non_positive_size_err = 'msgpack.decode_[^_]+_header: ' ..
         'non%-positive size'
+    local wrong_type_err = "msgpack.decode_[^_]+_header: 'char %*' expected"
 
     local decode_cases = {
         {
@@ -175,17 +176,17 @@ local function test_decode_array_map_header(test, s)
         {
             'data is nil',
             args = {nil, 1},
-            exp_err = 'expected cdata as 1 argument',
+            exp_err = wrong_type_err,
         },
         {
             'data is not cdata',
             args = {1, 1},
-            exp_err = 'expected cdata as 1 argument',
+            exp_err = wrong_type_err,
         },
         {
             'data with wrong cdata type',
             args = {box.tuple.new(), 1},
-            exp_err = "msgpack.decode_[^_]+_header: 'char %*' expected",
+            exp_err = wrong_type_err,
         },
         {
             'size has wrong type',
diff --git a/test/app/msgpack.result b/test/app/msgpack.result
index 9fc42fc3c..105f503da 100644
--- a/test/app/msgpack.result
+++ b/test/app/msgpack.result
@@ -4,6 +4,9 @@ buffer = require 'buffer'
 msgpack = require 'msgpack'
 ---
 ...
+ffi = require 'ffi'
+---
+...
 -- Arguments check.
 buf = buffer.ibuf()
 ---
@@ -226,3 +229,18 @@ size = msgpack.encode({c = 3, d = 4}, buf)
 ---
 - {'c': 3, 'd': 4}
 ...
+-- Decode should accept both 'char *' and 'const char *'.
+buf:reset()
+---
+...
+size = msgpack.encode(100, buf)
+---
+...
+(msgpack.decode(ffi.cast('char *', buf.rpos), size))
+---
+- 100
+...
+(msgpack.decode(ffi.cast('const char *', buf.rpos), size))
+---
+- 100
+...
diff --git a/test/app/msgpack.test.lua b/test/app/msgpack.test.lua
index 0920fa507..de8fd4e37 100644
--- a/test/app/msgpack.test.lua
+++ b/test/app/msgpack.test.lua
@@ -1,5 +1,6 @@
 buffer = require 'buffer'
 msgpack = require 'msgpack'
+ffi = require 'ffi'
 
 -- Arguments check.
 buf = buffer.ibuf()
@@ -71,3 +72,9 @@ size = msgpack.encode({a = 1, b = 2}, buf)
 buf = buffer.ibuf()
 size = msgpack.encode({c = 3, d = 4}, buf)
 (msgpack.decode(buf.rpos, size))
+
+-- Decode should accept both 'char *' and 'const char *'.
+buf:reset()
+size = msgpack.encode(100, buf)
+(msgpack.decode(ffi.cast('char *', buf.rpos), size))
+(msgpack.decode(ffi.cast('const char *', buf.rpos), size))
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 6/7] msgpack: allow to pass 'const char *' into decode()

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> msgpack.decode() internally uses 'const char *' variable to
> decode msgpack, but somewhy expects only 'char *' as input.
> This commit allows to pass 'const char *' as well.

ok to push


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] [PATCH 7/7] Drop an unused function and class

[Vladislav Shpilevoy]

---
 src/lua/utils.h | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/src/lua/utils.h b/src/lua/utils.h
index 3d887a5ce..943840ec0 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -597,31 +597,6 @@ tarantool_lua_utils_init(struct lua_State *L);
 
 #if defined(__cplusplus)
 } /* extern "C" */
-
-#include "exception.h"
-#include <fiber.h>
-
-static inline void
-luaT_call_xc(lua_State *L, int nargs, int nreturns)
-{
-	if (luaT_call(L, nargs, nreturns) != 0)
-		diag_raise();
-}
-
-/**
- * Make a reference to an object on top of the Lua stack and
- * release it at the end of the scope.
- */
-struct LuarefGuard
-{
-	int ref;
-	bool is_active;
-
-	explicit LuarefGuard(int ref_arg) { ref = ref_arg; is_active = true; }
-	explicit LuarefGuard(struct lua_State *L) { ref = luaL_ref(L, LUA_REGISTRYINDEX); is_active = true; }
-	~LuarefGuard() { if (is_active) luaL_unref(tarantool_L, LUA_REGISTRYINDEX, ref); }
-};
-
 #endif /* defined(__cplusplus) */
 
 #endif /* TARANTOOL_LUA_UTILS_H_INCLUDED */
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 7/7] Drop an unused function and class

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/05/15 04:21]:
> ---
>  src/lua/utils.h | 25 -------------------------
>  1 file changed, 25 deletions(-)

ok to push


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32

[tarantool-patches] Re: [PATCH 0/7] swim lua preparation, again

[Vladislav Shpilevoy]

Pushed to the master.

On 15/05/2019 02:06, Vladislav Shpilevoy wrote:
> This patchset consists of patches quite independent and needed for proper
> functioning of SWIM Lua API. Most of them are refactoring, and a couple of bug
> fixes.
> 
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/swim-lua-preparation
> Issue: https://github.com/tarantool/tarantool/issues/3234
> 
> Vladislav Shpilevoy (7):
>   swim: drop swim_info() function
>   swim: encapsulate 'uint16' payload size inside swim.c
>   swim: do not rebind when new 'port' is 0
>   swim: set 'left' status in self on swim_quit()
>   msgpack: allow to pass 'struct ibuf *' into encode()
>   msgpack: allow to pass 'const char *' into decode()
>   Drop an unused function and class
> 
>  src/box/lua/merger.c              | 24 ++---------------
>  src/lib/swim/swim.c               | 29 ++++----------------
>  src/lib/swim/swim.h               |  9 ++-----
>  src/lib/swim/swim_transport_udp.c | 14 ++++++++--
>  src/lua/msgpack.c                 | 28 ++++++--------------
>  src/lua/utils.c                   | 43 ++++++++++++++++++++++++++++++
>  src/lua/utils.h                   | 44 +++++++++++++------------------
>  test/app-tap/msgpack.test.lua     |  7 ++---
>  test/app/msgpack.result           | 44 ++++++++++++++++++++++++++++++-
>  test/app/msgpack.test.lua         | 16 +++++++++++
>  test/unit/swim.c                  | 20 +++++++++-----
>  test/unit/swim.result             | 21 ++++++++-------
>  test/unit/swim_test_utils.c       | 12 ++++-----
>  test/unit/swim_test_utils.h       |  6 ++---
>  14 files changed, 187 insertions(+), 130 deletions(-)
> 
> -- 
> 2.20.1 (Apple Git-117)
> 
>