From: Florian Forster <octo@collectd.org>
Date: Fri, 12 May 2017 10:08:55 +0000 (+0200)
Subject: src/utils_cmd_putval.c: Fix multi-value PUTVAL.
X-Git-Tag: collectd-5.7.2~8^2
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=619adc097bbc8f26e846db9a6cdfcb0322ba9944;p=collectd.git

src/utils_cmd_putval.c: Fix multi-value PUTVAL.

vl.values was allocated outside of the loop and then copied into each
ret_putval->vl[]. This means that later values overwrote the values
stored in previous ret_putval->vl[].
---

diff --git a/src/utils_cmd_putval.c b/src/utils_cmd_putval.c
index 817f954b..f1fe6d76 100644
--- a/src/utils_cmd_putval.c
+++ b/src/utils_cmd_putval.c
@@ -138,14 +138,6 @@ cmd_status_t cmd_parse_putval(size_t argc, char **argv,
   type = NULL;
   type_instance = NULL;
 
-  vl.values_len = ds->ds_num;
-  vl.values = malloc(vl.values_len * sizeof(*vl.values));
-  if (vl.values == NULL) {
-    cmd_error(CMD_ERROR, err, "malloc failed.");
-    sfree(identifier_copy);
-    return (CMD_ERROR);
-  }
-
   ret_putval->raw_identifier = identifier_copy;
   if (ret_putval->raw_identifier == NULL) {
     cmd_error(CMD_ERROR, err, "malloc failed.");
@@ -177,33 +169,46 @@ cmd_status_t cmd_parse_putval(size_t argc, char **argv,
     /* else: cmd_parse_option did not find an option; treat this as a
      * value list. */
 
+    vl.values_len = ds->ds_num;
+    vl.values = calloc(vl.values_len, sizeof(*vl.values));
+    if (vl.values == NULL) {
+      cmd_error(CMD_ERROR, err, "malloc failed.");
+      result = CMD_ERROR;
+      break;
+    }
+
     status = parse_values(argv[i], &vl, ds);
     if (status != 0) {
       cmd_error(CMD_PARSE_ERROR, err, "Parsing the values string failed.");
       result = CMD_PARSE_ERROR;
+      vl.values_len = 0;
+      sfree(vl.values);
       break;
     }
 
-    tmp = (value_list_t *)realloc(ret_putval->vl, (ret_putval->vl_num + 1) *
-                                                      sizeof(*ret_putval->vl));
+    tmp = realloc(ret_putval->vl,
+                  (ret_putval->vl_num + 1) * sizeof(*ret_putval->vl));
     if (tmp == NULL) {
       cmd_error(CMD_ERROR, err, "realloc failed.");
       cmd_destroy_putval(ret_putval);
       result = CMD_ERROR;
+      vl.values_len = 0;
+      sfree(vl.values);
       break;
     }
 
     ret_putval->vl = tmp;
     ret_putval->vl_num++;
     memcpy(&ret_putval->vl[ret_putval->vl_num - 1], &vl, sizeof(vl));
+
+    /* pointer is now owned by ret_putval->vl[] */
+    vl.values_len = 0;
+    vl.values = NULL;
   } /* while (*buffer != 0) */
   /* Done parsing the options. */
 
-  if (result != CMD_OK) {
-    if (ret_putval->vl_num == 0)
-      sfree(vl.values);
+  if (result != CMD_OK)
     cmd_destroy_putval(ret_putval);
-  }
 
   return (result);
 } /* cmd_status_t cmd_parse_putval */
@@ -215,8 +220,7 @@ void cmd_destroy_putval(cmd_putval_t *putval) {
   sfree(putval->raw_identifier);
 
   for (size_t i = 0; i < putval->vl_num; ++i) {
-    if (i == 0) /* values is shared between all entries */
-      sfree(putval->vl[i].values);
+    sfree(putval->vl[i].values);
     meta_data_destroy(putval->vl[i].meta);
     putval->vl[i].meta = NULL;
   }