ctdb-protocol: Fix marshalling for ctdb_req_header
authorAmitay Isaacs <amitay@gmail.com>
Wed, 19 Jul 2017 01:12:08 +0000 (11:12 +1000)
committerMartin Schwenke <martins@samba.org>
Wed, 30 Aug 2017 12:59:25 +0000 (14:59 +0200)
Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>
12 files changed:
ctdb/client/client_connect.c
ctdb/protocol/protocol_api.h
ctdb/protocol/protocol_call.c
ctdb/protocol/protocol_control.c
ctdb/protocol/protocol_debug.c
ctdb/protocol/protocol_header.c
ctdb/protocol/protocol_message.c
ctdb/tests/src/fake_ctdbd.c
ctdb/tests/src/protocol_common_ctdb.c
ctdb/tests/src/protocol_common_ctdb.h
ctdb/tests/src/protocol_ctdb_compat_test.c
ctdb/tests/src/protocol_ctdb_test.c

index 34259dd2b27af3e5695e8d24ea5a192e20255928..6fd2c87da136305caddecbe68902194c082709a4 100644 (file)
@@ -165,9 +165,10 @@ static void client_read_handler(uint8_t *buf, size_t buflen,
        struct ctdb_client_context *client = talloc_get_type_abort(
                private_data, struct ctdb_client_context);
        struct ctdb_req_header hdr;
+       size_t np;
        int ret;
 
-       ret = ctdb_req_header_pull(buf, buflen, &hdr);
+       ret = ctdb_req_header_pull(buf, buflen, &hdr, &np);
        if (ret != 0) {
                DEBUG(DEBUG_WARNING, ("invalid header, ret=%d\n", ret));
                return;
index 9e47f2691d90bb57b384539a88c2ab0e3bc8be98..d6acefbcbcc5b24996043a1106f7357af8ba4c91 100644 (file)
@@ -81,10 +81,11 @@ void ctdb_req_header_fill(struct ctdb_req_header *h, uint32_t generation,
                          uint32_t operation, uint32_t destnode,
                          uint32_t srcnode, uint32_t reqid);
 
-size_t ctdb_req_header_len(struct ctdb_req_header *h);
-void ctdb_req_header_push(struct ctdb_req_header *h, uint8_t *buf);
+size_t ctdb_req_header_len(struct ctdb_req_header *in);
+void ctdb_req_header_push(struct ctdb_req_header *in, uint8_t *buf,
+                         size_t *npush);
 int ctdb_req_header_pull(uint8_t *buf, size_t buflen,
-                        struct ctdb_req_header *h);
+                        struct ctdb_req_header *out, size_t *npull);
 
 int ctdb_req_header_verify(struct ctdb_req_header *h, uint32_t operation);
 
index 111cdf25de0aee348dae442d751942123ea04787..4fdbe99328751eedc407e5bf8be1daf891b5c9b6 100644 (file)
@@ -96,7 +96,7 @@ int ctdb_req_call_push(struct ctdb_req_header *h, struct ctdb_req_call *c,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->flags = c->flags;
        wire->db_id = c->db_id;
@@ -138,7 +138,8 @@ int ctdb_req_call_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -185,7 +186,7 @@ int ctdb_reply_call_push(struct ctdb_req_header *h, struct ctdb_reply_call *c,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->status = c->status;
        wire->datalen = ctdb_tdb_data_len(&c->data);
@@ -219,7 +220,8 @@ int ctdb_reply_call_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -257,7 +259,7 @@ int ctdb_reply_error_push(struct ctdb_req_header *h, struct ctdb_reply_error *c,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->status = c->status;
        wire->msglen = ctdb_tdb_data_len(&c->msg);
@@ -291,7 +293,8 @@ int ctdb_reply_error_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -330,7 +333,7 @@ int ctdb_req_dmaster_push(struct ctdb_req_header *h, struct ctdb_req_dmaster *c,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->db_id = c->db_id;
        wire->rsn = c->rsn;
@@ -371,7 +374,8 @@ int ctdb_req_dmaster_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -419,7 +423,7 @@ int ctdb_reply_dmaster_push(struct ctdb_req_header *h,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->db_id = c->db_id;
        wire->rsn = c->rsn;
@@ -459,7 +463,8 @@ int ctdb_reply_dmaster_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
index c271852d984a8220d3da9ccc6461c2193f938d6f..a55dd1a082492421c02a1190b6b507c94c9181f6 100644 (file)
@@ -1779,7 +1779,7 @@ int ctdb_req_control_push(struct ctdb_req_header *h,
 {
        struct ctdb_req_control_wire *wire =
                (struct ctdb_req_control_wire *)buf;
-       size_t length;
+       size_t length, np;
 
        length = ctdb_req_control_len(h, request);
        if (*buflen < length) {
@@ -1788,7 +1788,7 @@ int ctdb_req_control_push(struct ctdb_req_header *h,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->opcode = request->opcode;
        wire->pad = request->pad;
@@ -1809,7 +1809,7 @@ int ctdb_req_control_pull(uint8_t *buf, size_t buflen,
 {
        struct ctdb_req_control_wire *wire =
                (struct ctdb_req_control_wire *)buf;
-       size_t length;
+       size_t length, np;
        int ret;
 
        length = offsetof(struct ctdb_req_control_wire, data);
@@ -1827,7 +1827,8 @@ int ctdb_req_control_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -1872,7 +1873,7 @@ int ctdb_reply_control_push(struct ctdb_req_header *h,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->status = reply->status;
 
@@ -1918,7 +1919,8 @@ int ctdb_reply_control_pull(uint8_t *buf, size_t buflen, uint32_t opcode,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
index 574f903b2c00d0ec0245aecbb87048b08c02a3d3..a448fad9895c426f4829c2bee4ff966283aee632 100644 (file)
@@ -593,11 +593,12 @@ void ctdb_packet_print(uint8_t *buf, size_t buflen, FILE *fp)
 {
        TALLOC_CTX *mem_ctx = talloc_new(NULL);
        struct ctdb_req_header h;
+       size_t np;
        int ret;
 
        fprintf(fp, "Buffer len:%zu\n", buflen);
 
-       ret = ctdb_req_header_pull(buf, buflen, &h);
+       ret = ctdb_req_header_pull(buf, buflen, &h, &np);
        if (ret != 0) {
                fprintf(fp, "Failed to parse ctdb packet header\n");
                return;
index 178065af1d8eb7d7319000cc763c87b94b565ab0..a6be7f515477f711ad32e3f7270e5d80920eacf0 100644 (file)
@@ -25,6 +25,7 @@
 
 #include "protocol.h"
 #include "protocol_api.h"
+#include "protocol_private.h"
 
 int ctdb_req_header_verify(struct ctdb_req_header *h, uint32_t operation)
 {
@@ -61,23 +62,108 @@ void ctdb_req_header_fill(struct ctdb_req_header *h, uint32_t generation,
        h->reqid = reqid;
 }
 
-size_t ctdb_req_header_len(struct ctdb_req_header *h)
+size_t ctdb_req_header_len(struct ctdb_req_header *in)
 {
-       return sizeof(struct ctdb_req_header);
+       return ctdb_uint32_len(&in->length) +
+               ctdb_uint32_len(&in->ctdb_magic) +
+               ctdb_uint32_len(&in->ctdb_version) +
+               ctdb_uint32_len(&in->generation) +
+               ctdb_uint32_len(&in->operation) +
+               ctdb_uint32_len(&in->destnode) +
+               ctdb_uint32_len(&in->srcnode) +
+               ctdb_uint32_len(&in->reqid);
 }
 
-void ctdb_req_header_push(struct ctdb_req_header *h, uint8_t *buf)
+void ctdb_req_header_push(struct ctdb_req_header *in, uint8_t *buf,
+                         size_t *npush)
 {
-       memcpy(buf, h, sizeof(struct ctdb_req_header));
+       size_t offset = 0, np;
+
+       ctdb_uint32_push(&in->length, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->ctdb_magic, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->ctdb_version, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->generation, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->operation, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->destnode, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->srcnode, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->reqid, buf+offset, &np);
+       offset += np;
+
+       *npush = offset;
 }
 
 int ctdb_req_header_pull(uint8_t *buf, size_t buflen,
-                        struct ctdb_req_header *h)
+                        struct ctdb_req_header *out, size_t *npull)
 {
-       if (buflen < sizeof(struct ctdb_req_header)) {
-               return EMSGSIZE;
+       size_t offset = 0, np;
+       int ret;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->length, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->ctdb_magic,
+                              &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->ctdb_version,
+                              &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->generation,
+                              &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->operation,
+                              &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->destnode, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->srcnode, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->reqid, &np);
+       if (ret != 0) {
+               return ret;
        }
+       offset += np;
 
-       memcpy(h, buf, sizeof(struct ctdb_req_header));
+       *npull = offset;
        return 0;
 }
index 54a8c5a2470d2913252607435023057e0ab1c42e..1c1c0018fede54fe385a22e591c403c58ab1b323 100644 (file)
@@ -299,7 +299,7 @@ int ctdb_req_message_push(struct ctdb_req_header *h,
 {
        struct ctdb_req_message_wire *wire =
                (struct ctdb_req_message_wire *)buf;
-       size_t length;
+       size_t length, np;
 
        length = ctdb_req_message_len(h, message);
        if (*buflen < length) {
@@ -308,7 +308,7 @@ int ctdb_req_message_push(struct ctdb_req_header *h,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->srvid = message->srvid;
        wire->datalen = ctdb_message_data_len(&message->data, message->srvid);
@@ -324,7 +324,7 @@ int ctdb_req_message_pull(uint8_t *buf, size_t buflen,
 {
        struct ctdb_req_message_wire *wire =
                (struct ctdb_req_message_wire *)buf;
-       size_t length;
+       size_t length, np;
        int ret;
 
        length = offsetof(struct ctdb_req_message_wire, data);
@@ -342,7 +342,8 @@ int ctdb_req_message_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
@@ -376,7 +377,7 @@ int ctdb_req_message_data_push(struct ctdb_req_header *h,
        }
 
        h->length = *buflen;
-       ctdb_req_header_push(h, (uint8_t *)&wire->hdr);
+       ctdb_req_header_push(h, (uint8_t *)&wire->hdr, &np);
 
        wire->srvid = message->srvid;
        wire->datalen = ctdb_tdb_data_len(&message->data);
@@ -410,7 +411,8 @@ int ctdb_req_message_data_pull(uint8_t *buf, size_t buflen,
        }
 
        if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h);
+               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
+                                          &np);
                if (ret != 0) {
                        return ret;
                }
index 2bb9b1e37e38c27d0f9f5df2717ad69d04e1a1cf..ef3f1c1b5723b4b7122f91fd87e49f4eda024e1e 100644 (file)
@@ -3013,9 +3013,10 @@ static void client_read_handler(uint8_t *buf, size_t buflen,
                req, struct client_state);
        struct ctdbd_context *ctdb = state->ctdb;
        struct ctdb_req_header header;
+       size_t np;
        int ret, i;
 
-       ret = ctdb_req_header_pull(buf, buflen, &header);
+       ret = ctdb_req_header_pull(buf, buflen, &header, &np);
        if (ret != 0) {
                return;
        }
@@ -3035,7 +3036,7 @@ static void client_read_handler(uint8_t *buf, size_t buflen,
                for (i=0; i<ctdb->node_map->num_nodes; i++) {
                        header.destnode = i;
 
-                       ctdb_req_header_push(&header, buf);
+                       ctdb_req_header_push(&header, buf, &np);
                        client_process_packet(req, buf, buflen);
                }
                return;
@@ -3050,7 +3051,7 @@ static void client_read_handler(uint8_t *buf, size_t buflen,
 
                        header.destnode = i;
 
-                       ctdb_req_header_push(&header, buf);
+                       ctdb_req_header_push(&header, buf, &np);
                        client_process_packet(req, buf, buflen);
                }
                return;
@@ -3069,7 +3070,7 @@ static void client_read_handler(uint8_t *buf, size_t buflen,
                return;
        }
 
-       ctdb_req_header_push(&header, buf);
+       ctdb_req_header_push(&header, buf, &np);
        client_process_packet(req, buf, buflen);
 }
 
@@ -3085,9 +3086,10 @@ static void client_process_packet(struct tevent_req *req,
                                  uint8_t *buf, size_t buflen)
 {
        struct ctdb_req_header header;
+       size_t np;
        int ret;
 
-       ret = ctdb_req_header_pull(buf, buflen, &header);
+       ret = ctdb_req_header_pull(buf, buflen, &header, &np);
        if (ret != 0) {
                return;
        }
index 6116ec5a4330336ab8a4db388dd9673f105b6d27..c8c75ed1c047619b6e4104f626c6485bc1654226 100644 (file)
  * Functions to fill and verify protocol structures
  */
 
+void fill_ctdb_req_header(struct ctdb_req_header *h)
+{
+       h->length = rand32();
+       h->ctdb_magic = rand32();
+       h->ctdb_version = rand32();
+       h->generation = rand32();
+       h->operation = rand32();
+       h->destnode = rand32();
+       h->srcnode = rand32();
+       h->reqid = rand32();
+}
+
 void verify_ctdb_req_header(struct ctdb_req_header *h,
                            struct ctdb_req_header *h2)
 {
-       verify_buffer(h, h2, sizeof(struct ctdb_req_header));
+       assert(h->length == h2->length);
+       assert(h->ctdb_magic == h2->ctdb_magic);
+       assert(h->ctdb_version == h2->ctdb_version);
+       assert(h->generation == h2->generation);
+       assert(h->operation == h2->operation);
+       assert(h->destnode == h2->destnode);
+       assert(h->srcnode == h2->srcnode);
+       assert(h->reqid == h2->reqid);
 }
 
 void fill_ctdb_req_call(TALLOC_CTX *mem_ctx, struct ctdb_req_call *c)
index 39a3e13c14c21d353d94b030bb87056a0eb0ed4d..987af347f1ae05c827b011ff1d1b09b97f2972df 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "protocol/protocol.h"
 
+void fill_ctdb_req_header(struct ctdb_req_header *h);
 void verify_ctdb_req_header(struct ctdb_req_header *h,
                            struct ctdb_req_header *h2);
 
index 4047f821f7fa7b2f8ea0999e91a8a2c6923c2afb..581759700ce4f517598d79c16277d494b1baf892 100644 (file)
 
 #include "protocol/protocol_basic.c"
 #include "protocol/protocol_types.c"
+#include "protocol/protocol_header.c"
 
 #include "tests/src/protocol_common.h"
+#include "tests/src/protocol_common_ctdb.h"
 
 #define COMPAT_TEST_FUNC(NAME)         test_ ##NAME## _compat
 #define OLD_LEN_FUNC(NAME)             NAME## _len_old
@@ -202,6 +204,31 @@ static void COMPAT_TEST_FUNC(NAME)(uint64_t srvid) \
        talloc_free(mem_ctx); \
 }
 
+
+static size_t ctdb_req_header_len_old(struct ctdb_req_header *in)
+{
+        return sizeof(struct ctdb_req_header);
+}
+
+static void ctdb_req_header_push_old(struct ctdb_req_header *in, uint8_t *buf)
+{
+        memcpy(buf, in, sizeof(struct ctdb_req_header));
+}
+
+static int ctdb_req_header_pull_old(uint8_t *buf, size_t buflen,
+                                   struct ctdb_req_header *out)
+{
+        if (buflen < sizeof(struct ctdb_req_header)) {
+                return EMSGSIZE;
+        }
+
+        memcpy(out, buf, sizeof(struct ctdb_req_header));
+        return 0;
+}
+
+
+COMPAT_CTDB1_TEST(struct ctdb_req_header, ctdb_req_header);
+
 int main(int argc, char *argv[])
 {
        if (argc == 2) {
@@ -209,5 +236,7 @@ int main(int argc, char *argv[])
                srandom(seed);
        }
 
+       COMPAT_TEST_FUNC(ctdb_req_header)();
+
        return 0;
 }
index fccfe5524a8c5d9ca381f3410c4ef249270d2dc5..744f9e4ecc583687643799b7ee36fd2da770f505 100644 (file)
@@ -278,38 +278,7 @@ static void TEST_FUNC(NAME)(uint64_t srvid) \
        talloc_free(mem_ctx); \
 }
 
-static void test_ctdb_req_header(void)
-{
-       TALLOC_CTX *mem_ctx;
-       uint8_t *pkt;
-       size_t pkt_len;
-       struct ctdb_req_header h, h2;
-       int ret;
-
-       printf("ctdb_req_header\n");
-       fflush(stdout);
-
-       mem_ctx = talloc_new(NULL);
-       assert(mem_ctx != NULL);
-
-       ctdb_req_header_fill(&h, GENERATION, OPERATION, DESTNODE, SRCNODE,
-                            REQID);
-
-       ret = ctdb_allocate_pkt(mem_ctx, ctdb_req_header_len(&h),
-                               &pkt, &pkt_len);
-       assert(ret == 0);
-       assert(pkt != NULL);
-       assert(pkt_len >= ctdb_req_header_len(&h));
-
-       ctdb_req_header_push(&h, pkt);
-
-       ret = ctdb_req_header_pull(pkt, pkt_len, &h2);
-       assert(ret == 0);
-
-       verify_ctdb_req_header(&h, &h2);
-
-       talloc_free(mem_ctx);
-}
+PROTOCOL_CTDB1_TEST(struct ctdb_req_header, ctdb_req_header);
 
 static void test_ctdb_req_call(void)
 {
@@ -708,7 +677,7 @@ int main(int argc, char *argv[])
                srandom(seed);
        }
 
-       test_ctdb_req_header();
+       TEST_FUNC(ctdb_req_header)();
 
        test_ctdb_req_call();
        test_ctdb_reply_call();