ctdb-protocol: Fix marshalling for ctdb_req_message_data

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

4 files changed, 116 insertions, 81 deletions

diff --git a/ctdb/protocol/protocol_message.c b/ctdb/protocol/protocol_message.c
index f254ed19408..19c1f162d0c 100644
--- a/ctdb/protocol/protocol_message.c
+++ b/ctdb/protocol/protocol_message.c
@@ -27,12 +27,6 @@
 #include "protocol_api.h"
 #include "protocol_private.h"
 
-struct ctdb_req_message_wire {
-	struct ctdb_req_header hdr;
-	uint64_t srvid;
-	uint32_t datalen;
-	uint8_t data[1];
-};
 
 static size_t ctdb_message_data_len(union ctdb_message_data *mdata,
 				    uint64_t srvid)
@@ -403,30 +397,33 @@ int ctdb_req_message_pull(uint8_t *buf, size_t buflen,
 size_t ctdb_req_message_data_len(struct ctdb_req_header *h,
 				 struct ctdb_req_message_data *c)
 {
-	return offsetof(struct ctdb_req_message_wire, data) +
-		ctdb_tdb_data_len(&c->data);
+	return ctdb_req_header_len(h) +
+		ctdb_uint64_len(&c->srvid) +
+		ctdb_tdb_datan_len(&c->data);
 }
 
 int ctdb_req_message_data_push(struct ctdb_req_header *h,
-			       struct ctdb_req_message_data *message,
+			       struct ctdb_req_message_data *c,
 			       uint8_t *buf, size_t *buflen)
 {
-	struct ctdb_req_message_wire *wire =
-		(struct ctdb_req_message_wire *)buf;
-	size_t length, np;
+	size_t offset = 0, np;
+	size_t length;
 
-	length = ctdb_req_message_data_len(h, message);
+	length = ctdb_req_message_data_len(h, c);
 	if (*buflen < length) {
 		*buflen = length;
 		return EMSGSIZE;
 	}
 
 	h->length = *buflen;
-	ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
+	ctdb_req_header_push(h, buf+offset, &np);
+	offset += np;
 
-	wire->srvid = message->srvid;
-	wire->datalen = ctdb_tdb_data_len(&message->data);
-	ctdb_tdb_data_push(&message->data, wire->data, &np);
+	ctdb_uint64_push(&c->srvid, buf+offset, &np);
+	offset += np;
+
+	ctdb_tdb_datan_push(&c->data, buf+offset, &np);
+	offset += np;
 
 	return 0;
 }
@@ -436,40 +433,32 @@ int ctdb_req_message_data_pull(uint8_t *buf, size_t buflen,
 			       TALLOC_CTX *mem_ctx,
 			       struct ctdb_req_message_data *c)
 {
-	struct ctdb_req_message_wire *wire =
-		(struct ctdb_req_message_wire *)buf;
-	size_t length, np;
+	struct ctdb_req_header header;
+	size_t offset = 0, np;
 	int ret;
 
-	length = offsetof(struct ctdb_req_message_wire, data);
-	if (buflen < length) {
-		return EMSGSIZE;
-	}
-	if (wire->datalen > buflen) {
-		return EMSGSIZE;
-	}
-	if (length + wire->datalen < length) {
-		return EMSGSIZE;
-	}
-	if (buflen < length + wire->datalen) {
-		return EMSGSIZE;
+	ret = ctdb_req_header_pull(buf+offset, buflen-offset, &header, &np);
+	if (ret != 0) {
+		return ret;
 	}
+	offset += np;
 
 	if (h != NULL) {
-		ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
-					   &np);
-		if (ret != 0) {
-			return ret;
-		}
+		*h = header;
 	}
 
-	c->srvid = wire->srvid;
+	ret = ctdb_uint64_pull(buf+offset, buflen-offset, &c->srvid, &np);
+	if (ret != 0) {
+		return ret;
+	}
+	offset += np;
 
-	ret = ctdb_tdb_data_pull(wire->data, wire->datalen,
-				 mem_ctx, &c->data, &np);
+	ret = ctdb_tdb_datan_pull(buf+offset, buflen-offset,
+				  mem_ctx, &c->data, &np);
 	if (ret != 0) {
 		return ret;
 	}
+	offset += np;
 
 	return 0;
 }
diff --git a/ctdb/tests/cunit/protocol_test_101.sh b/ctdb/tests/cunit/protocol_test_101.sh
index d21d16b31e4..15c4150b3cb 100755
--- a/ctdb/tests/cunit/protocol_test_101.sh
+++ b/ctdb/tests/cunit/protocol_test_101.sh
@@ -53,7 +53,7 @@ output=$(
     generate_control_output "ctdb_reply_control"
     generate_message_output "ctdb_message_data"
     generate_message_output "ctdb_req_message"
-    echo "ctdb_req_message"
+    echo "ctdb_req_message_data"
 )
 
 ok "$output"
diff --git a/ctdb/tests/src/protocol_ctdb_compat_test.c b/ctdb/tests/src/protocol_ctdb_compat_test.c
index 0de62f42999..896c82e67d6 100644
--- a/ctdb/tests/src/protocol_ctdb_compat_test.c
+++ b/ctdb/tests/src/protocol_ctdb_compat_test.c
@@ -892,6 +892,13 @@ static int ctdb_reply_control_pull_old(uint8_t *buf, size_t buflen,
 	return 0;
 }
 
+struct ctdb_req_message_wire {
+	struct ctdb_req_header hdr;
+	uint64_t srvid;
+	uint32_t datalen;
+	uint8_t data[1];
+};
+
 static size_t ctdb_req_message_len_old(struct ctdb_req_header *h,
 				       struct ctdb_req_message *c)
 {
@@ -961,6 +968,80 @@ static int ctdb_req_message_pull_old(uint8_t *buf, size_t buflen,
 	return ret;
 }
 
+static size_t ctdb_req_message_data_len_old(struct ctdb_req_header *h,
+					    struct ctdb_req_message_data *c)
+{
+	return offsetof(struct ctdb_req_message_wire, data) +
+		ctdb_tdb_data_len(&c->data);
+}
+
+static int ctdb_req_message_data_push_old(struct ctdb_req_header *h,
+					  struct ctdb_req_message_data *c,
+					  uint8_t *buf, size_t *buflen)
+{
+	struct ctdb_req_message_wire *wire =
+		(struct ctdb_req_message_wire *)buf;
+	size_t length, np;
+
+	length = ctdb_req_message_data_len_old(h, c);
+	if (*buflen < length) {
+		*buflen = length;
+		return EMSGSIZE;
+	}
+
+	h->length = *buflen;
+	ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
+
+	wire->srvid = c->srvid;
+	wire->datalen = ctdb_tdb_data_len(&c->data);
+	ctdb_tdb_data_push(&c->data, wire->data, &np);
+
+	return 0;
+}
+
+static int ctdb_req_message_data_pull_old(uint8_t *buf, size_t buflen,
+					  struct ctdb_req_header *h,
+					  TALLOC_CTX *mem_ctx,
+					  struct ctdb_req_message_data *c)
+{
+	struct ctdb_req_message_wire *wire =
+		(struct ctdb_req_message_wire *)buf;
+	size_t length, np;
+	int ret;
+
+	length = offsetof(struct ctdb_req_message_wire, data);
+	if (buflen < length) {
+		return EMSGSIZE;
+	}
+	if (wire->datalen > buflen) {
+		return EMSGSIZE;
+	}
+	if (length + wire->datalen < length) {
+		return EMSGSIZE;
+	}
+	if (buflen < length + wire->datalen) {
+		return EMSGSIZE;
+	}
+
+	if (h != NULL) {
+		ret = ctdb_req_header_pull_old((uint8_t *)&wire->hdr, buflen,
+					       h);
+		if (ret != 0) {
+			return ret;
+		}
+	}
+
+	c->srvid = wire->srvid;
+
+	ret = ctdb_tdb_data_pull(wire->data, wire->datalen,
+				 mem_ctx, &c->data, &np);
+	if (ret != 0) {
+		return ret;
+	}
+
+	return 0;
+}
+
 
 COMPAT_CTDB1_TEST(struct ctdb_req_header, ctdb_req_header);
 
@@ -974,6 +1055,7 @@ COMPAT_CTDB5_TEST(struct ctdb_req_control, ctdb_req_control, CTDB_REQ_CONTROL);
 COMPAT_CTDB6_TEST(struct ctdb_reply_control, ctdb_reply_control, CTDB_REPLY_CONTROL);
 
 COMPAT_CTDB7_TEST(struct ctdb_req_message, ctdb_req_message, CTDB_REQ_MESSAGE);
+COMPAT_CTDB4_TEST(struct ctdb_req_message_data, ctdb_req_message_data, CTDB_REQ_MESSAGE);
 
 #define NUM_CONTROLS	151
 
@@ -1026,6 +1108,7 @@ int main(int argc, char *argv[])
 	for (i=0; i<ARRAY_SIZE(test_srvid); i++) {
 		COMPAT_TEST_FUNC(ctdb_req_message)(test_srvid[i]);
 	}
+	COMPAT_TEST_FUNC(ctdb_req_message_data)();
 
 	return 0;
 }
diff --git a/ctdb/tests/src/protocol_ctdb_test.c b/ctdb/tests/src/protocol_ctdb_test.c
index 31fe5c794e1..e563b31812b 100644
--- a/ctdb/tests/src/protocol_ctdb_test.c
+++ b/ctdb/tests/src/protocol_ctdb_test.c
@@ -302,45 +302,8 @@ PROTOCOL_CTDB6_TEST(struct ctdb_reply_control, ctdb_reply_control,
 PROTOCOL_CTDB3_TEST(union ctdb_message_data, ctdb_message_data);
 PROTOCOL_CTDB7_TEST(struct ctdb_req_message, ctdb_req_message,
 			CTDB_REQ_MESSAGE);
-
-static void test_ctdb_req_message_data(void)
-{
-	TALLOC_CTX *mem_ctx;
-	uint8_t *pkt;
-	size_t datalen, pkt_len, len;
-	int ret;
-	struct ctdb_req_header h, h2;
-	struct ctdb_req_message_data c, c2;
-
-	printf("ctdb_req_message\n");
-	fflush(stdout);
-
-	mem_ctx = talloc_new(NULL);
-	assert(mem_ctx != NULL);
-
-	ctdb_req_header_fill(&h, GENERATION, CTDB_REQ_MESSAGE,
-			     DESTNODE, SRCNODE, REQID);
-
-	fill_ctdb_req_message_data(mem_ctx, &c);
-	datalen = ctdb_req_message_data_len(&h, &c);
-	ret = ctdb_allocate_pkt(mem_ctx, datalen, &pkt, &pkt_len);
-	assert(ret == 0);
-	assert(pkt != NULL);
-	assert(pkt_len >= datalen);
-	len = 0;
-	ret = ctdb_req_message_data_push(&h, &c, pkt, &len);
-	assert(ret == EMSGSIZE);
-	assert(len == datalen);
-	ret = ctdb_req_message_data_push(&h, &c, pkt, &pkt_len);
-	assert(ret == 0);
-	ret = ctdb_req_message_data_pull(pkt, pkt_len, &h2, mem_ctx, &c2);
-	assert(ret == 0);
-	verify_ctdb_req_header(&h, &h2);
-	assert(h2.length == pkt_len);
-	verify_ctdb_req_message_data(&c, &c2);
-
-	talloc_free(mem_ctx);
-}
+PROTOCOL_CTDB4_TEST(struct ctdb_req_message_data, ctdb_req_message_data,
+			CTDB_REQ_MESSAGE);
 
 int main(int argc, char *argv[])
 {
@@ -401,7 +364,7 @@ int main(int argc, char *argv[])
 	for (i=0; i<ARRAY_SIZE(test_srvid); i++) {
 		TEST_FUNC(ctdb_req_message)(test_srvid[i]);
 	}
-	test_ctdb_req_message_data();
+	TEST_FUNC(ctdb_req_message_data)();
 
 	return 0;
 }