From fe4af1c0492b00fed423211d465798bf926da6b1 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Sun, 11 Sep 2016 18:54:48 -0400 Subject: [PATCH] Address review comments: - Add meta_data_as_string, - Use utility function. - Leak fixes. - Minor optimization. --- src/daemon/meta_data.c | 65 ++++++++++++++++++++++++++ src/daemon/meta_data.h | 5 ++ src/target_set.c | 122 ++++++++++++------------------------------------- 3 files changed, 100 insertions(+), 92 deletions(-) diff --git a/src/daemon/meta_data.c b/src/daemon/meta_data.c index 9e4fd07e..d26570e9 100644 --- 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 0398c546..f4a4f212 100644 --- 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 55ce2283..95f03173 100644 --- 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) \ -- 2.11.0