Address review comments:

- Add meta_data_as_string,
- Use utility function.
- Leak fixes.
- Minor optimization.

diff --git a/src/daemon/meta_data.c b/src/daemon/meta_data.c
index 9e4fd07..d26570e 100644 (file)
--- a/src/daemon/meta_data.c
+++ b/src/daemon/meta_data.c
@@ -26,9 +26,12 @@
 
 #include "collectd.h"
 
+#include "common.h"
 #include "plugin.h"
 #include "meta_data.h"
 
+#define MD_MAX_NONSTRING_CHARS 128
+
 /*
  * Data types
  */
@@ -729,4 +732,66 @@ int meta_data_get_boolean (meta_data_t *md, /* {{{ */
   return (0);
 } /* }}} int meta_data_get_boolean */
 
+int meta_data_as_string (meta_data_t *md, /* {{{ */
+    const char *key, char **value)
+{
+  meta_entry_t *e;
+  char *actual;
+  char buffer[MD_MAX_NONSTRING_CHARS];  /* For non-string types. */
+  char *temp;
+
+  if ((md == NULL) || (key == NULL) || (value == NULL))
+    return (-EINVAL);
+
+  pthread_mutex_lock (&md->lock);
+
+  e = md_entry_lookup (md, key);
+  if (e == NULL)
+  {
+    pthread_mutex_unlock (&md->lock);
+    return (-ENOENT);
+  }
+
+  switch (e->type)
+  {
+    case MD_TYPE_STRING:
+      actual = e->value.mv_string;
+      break;
+    case MD_TYPE_SIGNED_INT:
+      ssnprintf (buffer, sizeof (buffer), "%"PRIi64, e->value.mv_signed_int);
+      actual = buffer;
+      break;
+    case MD_TYPE_UNSIGNED_INT:
+      ssnprintf (buffer, sizeof (buffer), "%"PRIu64, e->value.mv_unsigned_int);
+      actual = buffer;
+      break;
+    case MD_TYPE_DOUBLE:
+      ssnprintf (buffer, sizeof (buffer), GAUGE_FORMAT, e->value.mv_double);
+      actual = buffer;
+      break;
+    case MD_TYPE_BOOLEAN:
+      actual = e->value.mv_boolean ? "true" : "false";
+      break;
+    default:
+      pthread_mutex_unlock (&md->lock);
+      ERROR ("meta_data_as_string: unknown type %d for key `%s'",
+          e->type, e->key);
+      return (-ENOENT);
+  }
+
+  temp = md_strdup (actual);
+  if (temp == NULL)
+  {
+    pthread_mutex_unlock (&md->lock);
+    ERROR ("meta_data_as_string: md_strdup failed for key `%s'.", e->key);
+    return (-ENOMEM);
+  }
+
+  pthread_mutex_unlock (&md->lock);
+
+  *value = temp;
+
+  return (0);
+} /* }}} int meta_data_as_string */
+
 /* vim: set sw=2 sts=2 et fdm=marker : */

diff --git a/src/daemon/meta_data.h b/src/daemon/meta_data.h
index 0398c54..f4a4f21 100644 (file)
--- a/src/daemon/meta_data.h
+++ b/src/daemon/meta_data.h
@@ -84,5 +84,10 @@ int meta_data_get_boolean (meta_data_t *md,
     const char *key,
     _Bool *value);
 
+/* Returns the value as a string, regardless of the type. */
+int meta_data_as_string (meta_data_t *md,
+    const char *key,
+    char **value);
+
 #endif /* META_DATA_H */
 /* vim: set sw=2 sts=2 et : */

diff --git a/src/target_set.c b/src/target_set.c
index 55ce228..95f0317 100644 (file)
--- a/src/target_set.c
+++ b/src/target_set.c
@@ -145,29 +145,24 @@ static int ts_config_add_meta_delete (ts_key_list_t **dest, /* {{{ */
 {
   ts_key_list_t *entry = NULL;
 
-  if ((ci->values_num != 1)
-      || (ci->values[0].type != OCONFIG_TYPE_STRING))
+  entry = calloc (1, sizeof (*entry));
+  if (entry == NULL)
   {
-    ERROR ("ts_config_add_meta_delete: The %s option requires "
-        "exactly one string argument.", ci->key);
-    return (-1);
+    ERROR ("ts_config_add_meta_delete: calloc failed.");
+    return (-ENOMEM);
   }
 
-  if (strlen (ci->values[0].value.string) == 0)
+  if (cf_util_get_string (ci, &entry->key) != 0)
+    return (-1);  /* An error has already been reported. */
+
+  if (strlen (entry->key) == 0)
   {
     ERROR ("Target `set': The `%s' option does not accept empty string as "
         "first argument.", ci->key);
+    sfree (entry->key);
     return (-1);
   }
 
-  entry = calloc (1, sizeof (*entry));
-  if (entry == NULL)
-  {
-    ERROR ("ts_config_add_meta_delete: calloc failed.");
-    return (-ENOMEM);
-  }
-
-  entry->key = sstrdup (ci->values[0].value.string);
   entry->next = *dest;
   *dest = entry;
 
@@ -178,12 +173,13 @@ static void ts_subst (char *dest, size_t size, const char *string, /* {{{ */
     const value_list_t *vl)
 {
   char temp[DATA_MAX_NAME_LEN];
-  int meta_entries;
-  char **meta_toc;
 
   /* Initialize the field with the template. */
   sstrncpy (dest, string, size);
 
+  if (strchr (dest, '%') == NULL)
+    return;
+
 #define REPLACE_FIELD(t, v) \
   if (subst_string (temp, sizeof (temp), dest, t, v) != NULL) \
     sstrncpy (dest, temp, size);
@@ -193,87 +189,25 @@ static void ts_subst (char *dest, size_t size, const char *string, /* {{{ */
   REPLACE_FIELD ("%{type}", vl->type);
   REPLACE_FIELD ("%{type_instance}", vl->type_instance);
 
-  meta_entries = meta_data_toc (vl->meta, &meta_toc);
-  for (int i = 0; i < meta_entries; i++)
+  if (vl->meta != NULL)
   {
-    char meta_name[DATA_MAX_NAME_LEN];
-    char value_str[DATA_MAX_NAME_LEN];
-    int meta_type;
-    const char *key = meta_toc[i];
-
-    ssnprintf (meta_name, sizeof (meta_name), "%%{meta:%s}", key);
-
-    meta_type = meta_data_type (vl->meta, key);
-    switch (meta_type)
+    char **meta_toc;
+    int meta_entries = meta_data_toc (vl->meta, &meta_toc);
+    for (int i = 0; i < meta_entries; i++)
     {
-      case MD_TYPE_STRING:
-        {
-          char *meta_value;
-          if (meta_data_get_string (vl->meta, key, &meta_value))
-          {
-            ERROR ("Target `set': Unable to get string metadata value `%s'.",
-                key);
-            continue;
-          }
-          sstrncpy (value_str, meta_value, sizeof (value_str));
-        }
-        break;
-      case MD_TYPE_SIGNED_INT:
-        {
-          int64_t meta_value;
-          if (meta_data_get_signed_int (vl->meta, key, &meta_value))
-          {
-            ERROR ("Target `set': Unable to get signed int metadata value "
-                "`%s'.", key);
-            continue;
-          }
-          ssnprintf (value_str, sizeof (value_str), "%"PRIi64, meta_value);
-        }
-        break;
-      case MD_TYPE_UNSIGNED_INT:
-        {
-          uint64_t meta_value;
-          if (meta_data_get_unsigned_int (vl->meta, key, &meta_value))
-          {
-            ERROR ("Target `set': Unable to get unsigned int metadata value "
-                "`%s'.", key);
-            continue;
-          }
-          ssnprintf (value_str, sizeof (value_str), "%"PRIu64, meta_value);
-        }
-        break;
-      case MD_TYPE_DOUBLE:
-        {
-          double meta_value;
-          if (meta_data_get_double (vl->meta, key, &meta_value))
-          {
-            ERROR ("Target `set': Unable to get double metadata value `%s'.",
-                key);
-            continue;
-          }
-          ssnprintf (value_str, sizeof (value_str), GAUGE_FORMAT, meta_value);
-        }
-        break;
-      case MD_TYPE_BOOLEAN:
-        {
-          _Bool meta_value;
-          if (meta_data_get_boolean (vl->meta, key, &meta_value))
-          {
-            ERROR ("Target `set': Unable to get boolean metadata value `%s'.",
-                key);
-            continue;
-          }
-          sstrncpy (value_str, meta_value ? "true" : "false",
-              sizeof (value_str));
-        }
-        break;
-      default:
-        ERROR ("Target `set': Unable to retrieve metadata type for `%s'.",
-            key);
+      char meta_name[DATA_MAX_NAME_LEN];
+      char *value_str;
+      const char *key = meta_toc[i];
+
+      ssnprintf (meta_name, sizeof (meta_name), "%%{meta:%s}", key);
+      if (meta_data_as_string (vl->meta, key, &value_str) != 0)
         continue;
+
+      REPLACE_FIELD (meta_name, value_str);
+      sfree (value_str);
     }
 
-    REPLACE_FIELD (meta_name, value_str);
+    strarray_free (meta_toc, (size_t) meta_entries);
   }
 } /* }}} int ts_subst */
 
@@ -447,6 +381,7 @@ static int ts_invoke (const data_set_t *ds, value_list_t *vl, /* {{{ */
       {
         ERROR ("Target `set': Unable to get replacement metadata value `%s'.",
             key);
+        strarray_free (meta_toc, (size_t) meta_entries);
         return (status);
       }
 
@@ -460,9 +395,12 @@ static int ts_invoke (const data_set_t *ds, value_list_t *vl, /* {{{ */
       if (status)
       {
         ERROR ("Target `set': Unable to set metadata value `%s'.", key);
+        strarray_free (meta_toc, (size_t) meta_entries);
         return (status);
       }
     }
+
+    strarray_free (meta_toc, (size_t) meta_entries);
   }
 
 #define SUBST_FIELD(f) \