ctdb-protocol: Fix marshalling for ctdb_var_list

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 c8edaf740f095617f39d99f7310ae71f72bf9562..9122fcbf117b507f6c414ce16a8ec5f5cec3a600 100644 (file)
--- a/ctdb/protocol/protocol_control.c
+++ b/ctdb/protocol/protocol_control.c
@@ -1472,7 +1472,7 @@ static void ctdb_reply_control_data_push(struct ctdb_reply_control_data *cd,
                break;
 
        case CTDB_CONTROL_LIST_TUNABLES:
-               ctdb_var_list_push(cd->data.tun_var_list, buf);
+               ctdb_var_list_push(cd->data.tun_var_list, buf, &np);
                break;
 
        case CTDB_CONTROL_GET_ALL_TUNABLES:
@@ -1644,7 +1644,7 @@ static int ctdb_reply_control_data_pull(uint8_t *buf, size_t buflen,
 
        case CTDB_CONTROL_LIST_TUNABLES:
                ret = ctdb_var_list_pull(buf, buflen, mem_ctx,
-                                        &cd->data.tun_var_list);
+                                        &cd->data.tun_var_list, &np);
                break;
 
        case CTDB_CONTROL_GET_ALL_TUNABLES:

diff --git a/ctdb/protocol/protocol_private.h b/ctdb/protocol/protocol_private.h
index 1afe5668bb764020798fa66361dd5aab6da1bd8b..1a01322ac307f2af5ed756179f9986f0d3aa3dcd 100644 (file)
--- a/ctdb/protocol/protocol_private.h
+++ b/ctdb/protocol/protocol_private.h
@@ -198,10 +198,10 @@ int ctdb_node_flag_change_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
                               struct ctdb_node_flag_change **out,
                               size_t *npull);
 
-size_t ctdb_var_list_len(struct ctdb_var_list *var_list);
-void ctdb_var_list_push(struct ctdb_var_list *var_list, uint8_t *buf);
+size_t ctdb_var_list_len(struct ctdb_var_list *in);
+void ctdb_var_list_push(struct ctdb_var_list *in, uint8_t *buf, size_t *npush);
 int ctdb_var_list_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
-                      struct ctdb_var_list **out);
+                      struct ctdb_var_list **out, size_t *npull);
 
 size_t ctdb_tunable_list_len(struct ctdb_tunable_list *tun_list);
 void ctdb_tunable_list_push(struct ctdb_tunable_list *tun_list, uint8_t *buf);

diff --git a/ctdb/protocol/protocol_types.c b/ctdb/protocol/protocol_types.c
index 537ea62084cc05cde17e1b48f207b02b149928b7..c33c9495c900d1b353a9aba4e63cd55d3e81453e 100644 (file)
--- a/ctdb/protocol/protocol_types.c
+++ b/ctdb/protocol/protocol_types.c
@@ -2270,95 +2270,110 @@ fail:
        return ret;
 }
 
-struct ctdb_var_list_wire {
-       uint32_t length;
-       char list_str[1];
-};
-
-size_t ctdb_var_list_len(struct ctdb_var_list *var_list)
+size_t ctdb_var_list_len(struct ctdb_var_list *in)
 {
+       uint32_t u32 = 0;
        int i;
-       size_t len = sizeof(uint32_t);
 
-       for (i=0; i<var_list->count; i++) {
-               len += strlen(var_list->var[i]) + 1;
+       for (i=0; i<in->count; i++) {
+               u32 += ctdb_string_len(&in->var[i]);
        }
-       return len;
+
+       return ctdb_uint32_len(&u32) + u32;
 }
 
-void ctdb_var_list_push(struct ctdb_var_list *var_list, uint8_t *buf)
+void ctdb_var_list_push(struct ctdb_var_list *in, uint8_t *buf, size_t *npush)
 {
-       struct ctdb_var_list_wire *wire = (struct ctdb_var_list_wire *)buf;
-       int i, n;
-       size_t offset = 0;
+       size_t offset = 0, np;
+       uint32_t u32;
+       int i;
+       uint8_t sep = ':';
 
-       if (var_list->count > 0) {
-               n = sprintf(wire->list_str, "%s", var_list->var[0]);
-               offset += n;
-       }
-       for (i=1; i<var_list->count; i++) {
-               n = sprintf(&wire->list_str[offset], ":%s", var_list->var[i]);
-               offset += n;
+       /* The length only corresponds to the payload size */
+       u32 = ctdb_var_list_len(in);
+       u32 -= ctdb_uint32_len(&u32);
+
+       ctdb_uint32_push(&u32, buf+offset, &np);
+       offset += np;
+
+       /* The variables are separated by ':' and the complete string is null
+        * terminated.
+        */
+       for (i=0; i<in->count; i++) {
+               ctdb_string_push(&in->var[i], buf+offset, &np);
+               offset += np;
+
+               if (i < in->count - 1) {
+                       /* Replace '\0' with ':' */
+                       ctdb_uint8_push(&sep, buf+offset-1, &np);
+               }
        }
-       wire->length = offset + 1;
+
+       *npush = offset;
 }
 
 int ctdb_var_list_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
-                      struct ctdb_var_list **out)
+                      struct ctdb_var_list **out, size_t *npull)
 {
-       struct ctdb_var_list *var_list = NULL;
-       struct ctdb_var_list_wire *wire = (struct ctdb_var_list_wire *)buf;
-       char *str, *s, *tok, *ptr;
-       const char **list;
+       struct ctdb_var_list *val;
+       const char *str, **list;
+       char *s, *tok, *ptr = NULL;
+       size_t offset = 0, np;
+       uint32_t u32;
+       int ret;
 
-       if (buflen < sizeof(uint32_t)) {
-               return EMSGSIZE;
-       }
-       if (wire->length > buflen) {
-               return EMSGSIZE;
-       }
-       if (sizeof(uint32_t) + wire->length < sizeof(uint32_t)) {
-               return EMSGSIZE;
+       val = talloc_zero(mem_ctx, struct ctdb_var_list);
+       if (val == NULL) {
+               return ENOMEM;
        }
-       if (buflen < sizeof(uint32_t) + wire->length) {
-               return EMSGSIZE;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset, &u32, &np);
+       if (ret != 0) {
+               goto fail;
        }
+       offset += np;
 
-       str = talloc_strndup(mem_ctx, (char *)wire->list_str, wire->length);
-       if (str == NULL) {
-               return ENOMEM;
+       if (buflen-offset < u32) {
+               ret = EMSGSIZE;
+               goto fail;
        }
 
-       var_list = talloc_zero(mem_ctx, struct ctdb_var_list);
-       if (var_list == NULL) {
+       ret = ctdb_string_pull(buf+offset, u32, val, &str, &np);
+       if (ret != 0) {
                goto fail;
        }
+       offset += np;
 
-       s = str;
+       s = discard_const(str);
        while ((tok = strtok_r(s, ":", &ptr)) != NULL) {
-               s = NULL;
-               list = talloc_realloc(var_list, var_list->var, const char *,
-                                     var_list->count+1);
+               list = talloc_realloc(val, val->var, const char *,
+                                     val->count+1);
                if (list == NULL) {
+                       ret = ENOMEM;
                        goto fail;
                }
 
-               var_list->var = list;
-               var_list->var[var_list->count] = talloc_strdup(var_list, tok);
-               if (var_list->var[var_list->count] == NULL) {
+               val->var = list;
+
+               s = talloc_strdup(val, tok);
+               if (s == NULL) {
+                       ret = ENOMEM;
                        goto fail;
                }
-               var_list->count++;
+
+               val->var[val->count] = s;
+               val->count += 1;
+               s = NULL;
        }
 
-       talloc_free(str);
-       *out = var_list;
+       talloc_free(discard_const(str));
+       *out = val;
+       *npull = offset;
        return 0;
 
 fail:
-       talloc_free(str);
-       talloc_free(var_list);
-       return ENOMEM;
+       talloc_free(val);
+       return ret;
 }
 
 size_t ctdb_tunable_list_len(struct ctdb_tunable_list *tun_list)

diff --git a/ctdb/tests/src/protocol_types_compat_test.c b/ctdb/tests/src/protocol_types_compat_test.c
index e2e6d39fb0605b8b16f000aa80ffad517b470af9..e58c93067094d5c61c0691f07d801604e42140ba 100644 (file)
--- a/ctdb/tests/src/protocol_types_compat_test.c
+++ b/ctdb/tests/src/protocol_types_compat_test.c
@@ -825,6 +825,98 @@ static int ctdb_node_flag_change_pull_old(uint8_t *buf, size_t buflen,
        return 0;
 }
 
+struct ctdb_var_list_wire {
+       uint32_t length;
+       char list_str[1];
+};
+
+static size_t ctdb_var_list_len_old(struct ctdb_var_list *in)
+{
+       int i;
+       size_t len = sizeof(uint32_t);
+
+       for (i=0; i<in->count; i++) {
+               len += strlen(in->var[i]) + 1;
+       }
+       return len;
+}
+
+static void ctdb_var_list_push_old(struct ctdb_var_list *in, uint8_t *buf)
+{
+       struct ctdb_var_list_wire *wire = (struct ctdb_var_list_wire *)buf;
+       int i, n;
+       size_t offset = 0;
+
+       if (in->count > 0) {
+               n = sprintf(wire->list_str, "%s", in->var[0]);
+               offset += n;
+       }
+       for (i=1; i<in->count; i++) {
+               n = sprintf(&wire->list_str[offset], ":%s", in->var[i]);
+               offset += n;
+       }
+       wire->length = offset + 1;
+}
+
+static int ctdb_var_list_pull_old(uint8_t *buf, size_t buflen,
+                                 TALLOC_CTX *mem_ctx,
+                                 struct ctdb_var_list **out)
+{
+       struct ctdb_var_list *val = NULL;
+       struct ctdb_var_list_wire *wire = (struct ctdb_var_list_wire *)buf;
+       char *str, *s, *tok, *ptr;
+       const char **list;
+
+       if (buflen < sizeof(uint32_t)) {
+               return EMSGSIZE;
+       }
+       if (wire->length > buflen) {
+               return EMSGSIZE;
+       }
+       if (sizeof(uint32_t) + wire->length < sizeof(uint32_t)) {
+               return EMSGSIZE;
+       }
+       if (buflen < sizeof(uint32_t) + wire->length) {
+               return EMSGSIZE;
+       }
+
+       str = talloc_strndup(mem_ctx, (char *)wire->list_str, wire->length);
+       if (str == NULL) {
+               return ENOMEM;
+       }
+
+       val = talloc_zero(mem_ctx, struct ctdb_var_list);
+       if (val == NULL) {
+               goto fail;
+       }
+
+       s = str;
+       while ((tok = strtok_r(s, ":", &ptr)) != NULL) {
+               s = NULL;
+               list = talloc_realloc(val, val->var, const char *,
+                                     val->count+1);
+               if (list == NULL) {
+                       goto fail;
+               }
+
+               val->var = list;
+               val->var[val->count] = talloc_strdup(val, tok);
+               if (val->var[val->count] == NULL) {
+                       goto fail;
+               }
+               val->count++;
+       }
+
+       talloc_free(str);
+       *out = val;
+       return 0;
+
+fail:
+       talloc_free(str);
+       talloc_free(val);
+       return ENOMEM;
+}
+
 
 COMPAT_TYPE3_TEST(struct ctdb_statistics, ctdb_statistics);
 COMPAT_TYPE3_TEST(struct ctdb_vnn_map, ctdb_vnn_map);
@@ -844,6 +936,7 @@ COMPAT_TYPE3_TEST(ctdb_sock_addr, ctdb_sock_addr);
 COMPAT_TYPE3_TEST(struct ctdb_connection, ctdb_connection);
 COMPAT_TYPE3_TEST(struct ctdb_tunable, ctdb_tunable);
 COMPAT_TYPE3_TEST(struct ctdb_node_flag_change, ctdb_node_flag_change);
+COMPAT_TYPE3_TEST(struct ctdb_var_list, ctdb_var_list);
 
 int main(int argc, char *argv[])
 {
@@ -868,6 +961,7 @@ int main(int argc, char *argv[])
        COMPAT_TEST_FUNC(ctdb_connection)();
        COMPAT_TEST_FUNC(ctdb_tunable)();
        COMPAT_TEST_FUNC(ctdb_node_flag_change)();
+       COMPAT_TEST_FUNC(ctdb_var_list)();
 
        return 0;
 }

diff --git a/ctdb/tests/src/protocol_types_test.c b/ctdb/tests/src/protocol_types_test.c
index 4e2f7d347e7cc237475bc1702bf219a91d8b1412..6961c5704871d4a77d81ef1b32197e5d537d07bd 100644 (file)
--- a/ctdb/tests/src/protocol_types_test.c
+++ b/ctdb/tests/src/protocol_types_test.c
@@ -64,7 +64,7 @@ PROTOCOL_TYPE3_TEST(ctdb_sock_addr, ctdb_sock_addr);
 PROTOCOL_TYPE3_TEST(struct ctdb_connection, ctdb_connection);
 PROTOCOL_TYPE3_TEST(struct ctdb_tunable, ctdb_tunable);
 PROTOCOL_TYPE3_TEST(struct ctdb_node_flag_change, ctdb_node_flag_change);
-DEFINE_TEST(struct ctdb_var_list, ctdb_var_list);
+PROTOCOL_TYPE3_TEST(struct ctdb_var_list, ctdb_var_list);
 DEFINE_TEST(struct ctdb_tunable_list, ctdb_tunable_list);
 DEFINE_TEST(struct ctdb_tickle_list, ctdb_tickle_list);
 DEFINE_TEST(struct ctdb_addr_info, ctdb_addr_info);