ctdb-protocol: Fix marshalling for ctdb_reply_control

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

diff --git a/ctdb/protocol/protocol_control.c b/ctdb/protocol/protocol_control.c
index 4c2a086364e0428d0e6cc5aee9524cd2025eb4bd..50114ba980a9c21e6644ba12b62306ed7e0928c0 100644 (file)
--- a/ctdb/protocol/protocol_control.c
+++ b/ctdb/protocol/protocol_control.c
@@ -27,14 +27,6 @@
 #include "protocol_api.h"
 #include "protocol_private.h"
 
-struct ctdb_reply_control_wire {
-       struct ctdb_req_header hdr;
-       int32_t status;
-       uint32_t datalen;
-       uint32_t errorlen;
-       uint8_t data[1];
-};
-
 static size_t ctdb_req_control_data_len(struct ctdb_req_control_data *cd)
 {
        size_t len = 0;
@@ -1910,42 +1902,65 @@ int ctdb_req_control_pull(uint8_t *buf, size_t buflen,
 size_t ctdb_reply_control_len(struct ctdb_req_header *h,
                              struct ctdb_reply_control *c)
 {
-       return offsetof(struct ctdb_reply_control_wire, data) +
-               (c->status == 0 ?
-                       ctdb_reply_control_data_len(&c->rdata) :
-                       ctdb_string_len(&c->errmsg));
+       uint32_t dsize, esize;
+
+       if (c->status == 0) {
+               dsize = ctdb_reply_control_data_len(&c->rdata);
+               esize = 0;
+       } else {
+               dsize = 0;
+               esize = ctdb_string_len(&c->errmsg);
+       }
+
+       return ctdb_req_header_len(h) +
+               ctdb_int32_len(&c->status) +
+               ctdb_uint32_len(&dsize) +
+               ctdb_uint32_len(&esize) +
+               dsize + esize;
 }
 
 int ctdb_reply_control_push(struct ctdb_req_header *h,
-                           struct ctdb_reply_control *reply,
+                           struct ctdb_reply_control *c,
                            uint8_t *buf, size_t *buflen)
 {
-       struct ctdb_reply_control_wire *wire =
-               (struct ctdb_reply_control_wire *)buf;
-       size_t length, np;
+       size_t offset = 0, np;
+       size_t length;
+       uint32_t dsize, esize;
 
-       length = ctdb_reply_control_len(h, reply);
+       length = ctdb_reply_control_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->status = reply->status;
+       ctdb_int32_push(&c->status, buf+offset, &np);
+       offset += np;
 
-       if (reply->status == 0) {
-               wire->datalen = ctdb_reply_control_data_len(&reply->rdata);
-               wire->errorlen = 0;
-               ctdb_reply_control_data_push(&reply->rdata, wire->data, &np);
+       if (c->status == 0) {
+               dsize = ctdb_reply_control_data_len(&c->rdata);
+               esize = 0;
        } else {
-               wire->datalen = 0;
-               wire->errorlen = ctdb_string_len(&reply->errmsg);
-               ctdb_string_push(&reply->errmsg, wire->data + wire->datalen,
-                                &np);
+               dsize = 0;
+               esize = ctdb_string_len(&c->errmsg);
        }
 
+       ctdb_uint32_push(&dsize, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&esize, buf+offset, &np);
+       offset += np;
+
+       if (c->status == 0) {
+               ctdb_reply_control_data_push(&c->rdata, buf+offset, &np);
+       } else {
+               ctdb_string_push(&c->errmsg, buf+offset, &np);
+       }
+       offset += np;
+
        return 0;
 }
 
@@ -1954,51 +1969,65 @@ int ctdb_reply_control_pull(uint8_t *buf, size_t buflen, uint32_t opcode,
                            TALLOC_CTX *mem_ctx,
                            struct ctdb_reply_control *c)
 {
-       struct ctdb_reply_control_wire *wire =
-               (struct ctdb_reply_control_wire *)buf;
-       size_t length, np;
+       struct ctdb_req_header header;
+       size_t offset = 0, np;
+       uint32_t dsize, esize;
        int ret;
 
-       length = offsetof(struct ctdb_reply_control_wire, data);
-       if (buflen < length) {
-               return EMSGSIZE;
+       ret = ctdb_req_header_pull(buf+offset, buflen-offset, &header, &np);
+       if (ret != 0) {
+               return ret;
        }
-       if (wire->datalen > buflen || wire->errorlen > buflen) {
-               return EMSGSIZE;
+       offset += np;
+
+       if (h != NULL) {
+               *h = header;
        }
-       if (length + wire->datalen < length) {
-               return EMSGSIZE;
+
+       ret = ctdb_int32_pull(buf+offset, buflen-offset, &c->status, &np);
+       if (ret != 0) {
+               return ret;
        }
-       if (length + wire->datalen + wire->errorlen < length) {
-               return EMSGSIZE;
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &dsize, &np);
+       if (ret != 0) {
+               return ret;
        }
-       if (buflen < length + wire->datalen + wire->errorlen) {
-               return EMSGSIZE;
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &esize, &np);
+       if (ret != 0) {
+               return ret;
        }
+       offset += np;
 
-       if (h != NULL) {
-               ret = ctdb_req_header_pull((uint8_t *)&wire->hdr, buflen, h,
-                                          &np);
+       c->errmsg = NULL;
+
+       if (c->status == 0) {
+               if (buflen-offset < dsize) {
+                       return EMSGSIZE;
+               }
+
+               ret = ctdb_reply_control_data_pull(buf+offset, dsize,
+                                                  opcode, mem_ctx, &c->rdata,
+                                                  &np);
                if (ret != 0) {
                        return ret;
                }
-       }
+               offset += np;
 
-       c->status = wire->status;
+       } else {
+               if (buflen-offset < esize) {
+                       return EMSGSIZE;
+               }
 
-       if (c->status != -1) {
-               ret = ctdb_reply_control_data_pull(wire->data, wire->datalen,
-                                                  opcode, mem_ctx,
-                                                  &c->rdata, &np);
+               ret = ctdb_string_pull(buf+offset, esize, mem_ctx, &c->errmsg,
+                                      &np);
                if (ret != 0) {
                        return ret;
                }
-       }
-
-       ret = ctdb_string_pull(wire->data + wire->datalen, wire->errorlen,
-                              mem_ctx, &c->errmsg, &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 9a177d2b179121fcdb8e7bd0845756cfca576c12..e6c5da891fdd6097bcb84c997c995a667939fc98 100755 (executable)
--- a/ctdb/tests/cunit/protocol_test_101.sh
+++ b/ctdb/tests/cunit/protocol_test_101.sh
@@ -4,13 +4,6 @@
 
 last_control=150
 
-control_output=$(
-    for i in $(seq 0 $last_control) ; do
-       echo -n "$i.. "
-    done
-    echo
-)
-
 generate_control_output ()
 {
     for i in $(seq 0 $last_control) ; do
@@ -28,8 +21,7 @@ output=$(
     generate_control_output "ctdb_req_control_data"
     generate_control_output "ctdb_reply_control_data"
     generate_control_output "ctdb_req_control"
-    echo "ctdb_reply_control"
-    echo "$control_output"
+    generate_control_output "ctdb_reply_control"
     echo "ctdb_req_message"
 )
 

diff --git a/ctdb/tests/src/protocol_ctdb_compat_test.c b/ctdb/tests/src/protocol_ctdb_compat_test.c
index c1593f70401eeb87c2133d97a6b4a7a4264708c8..960f131abb0550889d23487ac0a1cd1d3fa2474e 100644 (file)
--- a/ctdb/tests/src/protocol_ctdb_compat_test.c
+++ b/ctdb/tests/src/protocol_ctdb_compat_test.c
@@ -786,6 +786,111 @@ static int ctdb_req_control_pull_old(uint8_t *buf, size_t buflen,
        return 0;
 }
 
+struct ctdb_reply_control_wire {
+       struct ctdb_req_header hdr;
+       int32_t status;
+       uint32_t datalen;
+       uint32_t errorlen;
+       uint8_t data[1];
+};
+
+static size_t ctdb_reply_control_len_old(struct ctdb_req_header *h,
+                                        struct ctdb_reply_control *c)
+{
+       return offsetof(struct ctdb_reply_control_wire, data) +
+               (c->status == 0 ?
+                       ctdb_reply_control_data_len(&c->rdata) :
+                       ctdb_string_len(&c->errmsg));
+}
+
+static int ctdb_reply_control_push_old(struct ctdb_req_header *h,
+                                      struct ctdb_reply_control *c,
+                                      uint8_t *buf, size_t *buflen)
+{
+       struct ctdb_reply_control_wire *wire =
+               (struct ctdb_reply_control_wire *)buf;
+       size_t length, np;
+
+       length = ctdb_reply_control_len_old(h, c);
+       if (*buflen < length) {
+               *buflen = length;
+               return EMSGSIZE;
+       }
+
+       h->length = *buflen;
+       ctdb_req_header_push_old(h, (uint8_t *)&wire->hdr);
+
+       wire->status = c->status;
+
+       if (c->status == 0) {
+               wire->datalen = ctdb_reply_control_data_len(&c->rdata);
+               wire->errorlen = 0;
+               ctdb_reply_control_data_push(&c->rdata, wire->data, &np);
+       } else {
+               wire->datalen = 0;
+               wire->errorlen = ctdb_string_len(&c->errmsg);
+               ctdb_string_push(&c->errmsg, wire->data + wire->datalen, &np);
+       }
+
+       return 0;
+}
+
+static int ctdb_reply_control_pull_old(uint8_t *buf, size_t buflen,
+                                      uint32_t opcode,
+                                      struct ctdb_req_header *h,
+                                      TALLOC_CTX *mem_ctx,
+                                      struct ctdb_reply_control *c)
+{
+       struct ctdb_reply_control_wire *wire =
+               (struct ctdb_reply_control_wire *)buf;
+       size_t length, np;
+       int ret;
+
+       length = offsetof(struct ctdb_reply_control_wire, data);
+       if (buflen < length) {
+               return EMSGSIZE;
+       }
+       if (wire->datalen > buflen || wire->errorlen > buflen) {
+               return EMSGSIZE;
+       }
+       if (length + wire->datalen < length) {
+               return EMSGSIZE;
+       }
+       if (length + wire->datalen + wire->errorlen < length) {
+               return EMSGSIZE;
+       }
+       if (buflen < length + wire->datalen + wire->errorlen) {
+               return EMSGSIZE;
+       }
+
+       if (h != NULL) {
+               ret = ctdb_req_header_pull_old((uint8_t *)&wire->hdr, buflen,
+                                              h);
+               if (ret != 0) {
+                       return ret;
+               }
+       }
+
+       c->status = wire->status;
+
+       if (c->status != -1) {
+               ret = ctdb_reply_control_data_pull(wire->data, wire->datalen,
+                                                  opcode, mem_ctx,
+                                                  &c->rdata, &np);
+               if (ret != 0) {
+                       return ret;
+               }
+       }
+
+       ret = ctdb_string_pull(wire->data + wire->datalen, wire->errorlen,
+                              mem_ctx, &c->errmsg, &np);
+       if (ret != 0) {
+               return ret;
+       }
+
+       return 0;
+}
+
 
 COMPAT_CTDB1_TEST(struct ctdb_req_header, ctdb_req_header);
 
@@ -796,6 +901,7 @@ COMPAT_CTDB4_TEST(struct ctdb_req_dmaster, ctdb_req_dmaster, CTDB_REQ_DMASTER);
 COMPAT_CTDB4_TEST(struct ctdb_reply_dmaster, ctdb_reply_dmaster, CTDB_REPLY_DMASTER);
 
 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);
 
 #define NUM_CONTROLS   151
 
@@ -819,6 +925,9 @@ int main(int argc, char *argv[])
        for (opcode=0; opcode<NUM_CONTROLS; opcode++) {
                COMPAT_TEST_FUNC(ctdb_req_control)(opcode);
        }
+       for (opcode=0; opcode<NUM_CONTROLS; opcode++) {
+               COMPAT_TEST_FUNC(ctdb_reply_control)(opcode);
+       }
 
        return 0;
 }

diff --git a/ctdb/tests/src/protocol_ctdb_test.c b/ctdb/tests/src/protocol_ctdb_test.c
index 9636bb5cc3faa113f66d85052a124e059e2eff94..1579d154d52e73e3c74a716bb7d0e72966d0d37c 100644 (file)
--- a/ctdb/tests/src/protocol_ctdb_test.c
+++ b/ctdb/tests/src/protocol_ctdb_test.c
@@ -296,53 +296,8 @@ PROTOCOL_CTDB2_TEST(struct ctdb_reply_control_data, ctdb_reply_control_data);
 
 PROTOCOL_CTDB5_TEST(struct ctdb_req_control, ctdb_req_control,
                        CTDB_REQ_CONTROL);
-
-static void test_ctdb_reply_control(void)
-{
-       TALLOC_CTX *mem_ctx;
-       uint8_t *pkt;
-       size_t datalen, pkt_len, len;
-       int ret;
-       struct ctdb_req_header h, h2;
-       struct ctdb_reply_control c, c2;
-       uint32_t opcode;
-
-       printf("ctdb_reply_control\n");
-       fflush(stdout);
-
-       ctdb_req_header_fill(&h, GENERATION, CTDB_REPLY_CONTROL,
-                            DESTNODE, SRCNODE, REQID);
-
-       for (opcode=0; opcode<NUM_CONTROLS; opcode++) {
-               mem_ctx = talloc_new(NULL);
-               assert(mem_ctx != NULL);
-
-               printf("%u.. ", opcode);
-               fflush(stdout);
-               fill_ctdb_reply_control(mem_ctx, &c, opcode);
-               datalen = ctdb_reply_control_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_reply_control_push(&h, &c, pkt, &len);
-               assert(ret == EMSGSIZE);
-               assert(len == datalen);
-               ret = ctdb_reply_control_push(&h, &c, pkt, &pkt_len);
-               assert(ret == 0);
-               ret = ctdb_reply_control_pull(pkt, pkt_len, opcode, &h2, mem_ctx, &c2);
-               assert(ret == 0);
-               verify_ctdb_req_header(&h, &h2);
-               assert(h2.length == pkt_len);
-               verify_ctdb_reply_control(&c, &c2);
-
-               talloc_free(mem_ctx);
-       }
-
-       printf("\n");
-       fflush(stdout);
-}
+PROTOCOL_CTDB6_TEST(struct ctdb_reply_control, ctdb_reply_control,
+                       CTDB_REPLY_CONTROL);
 
 static void test_ctdb_req_message_data(void)
 {
@@ -410,7 +365,9 @@ int main(int argc, char *argv[])
        for (opcode=0; opcode<NUM_CONTROLS; opcode++) {
                TEST_FUNC(ctdb_req_control)(opcode);
        }
-       test_ctdb_reply_control();
+       for (opcode=0; opcode<NUM_CONTROLS; opcode++) {
+               TEST_FUNC(ctdb_reply_control)(opcode);
+       }
 
        test_ctdb_req_message_data();