From c7977d3b2ef51259f0375e30cbd3d22c00c2fdbf Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Fri, 9 Apr 2010 22:10:24 +0200 Subject: [PATCH] pinba plugin: Make the config parsing easier to read ... ... by using "cf_util_get_string". --- src/pinba.c | 136 +++++++++++++++++++++++++--------------------------- 1 file changed, 64 insertions(+), 72 deletions(-) diff --git a/src/pinba.c b/src/pinba.c index 46a2ca33..94e1092a 100644 --- a/src/pinba.c +++ b/src/pinba.c @@ -559,89 +559,72 @@ static void *collector_thread (void *arg) /* {{{ */ /* * Plugin declaration section */ - -static int config_set (char **var, const char *value) /* {{{ */ +static int pinba_config_view (const oconfig_item_t *ci) /* {{{ */ { - /* code from nginx plugin for collectd */ - if (*var != NULL) { - free (*var); - *var = NULL; + char *name = NULL; + char *host = NULL; + char *server = NULL; + char *script = NULL; + int status; + int i; + + status = cf_util_get_string (ci, &name); + if (status != 0) + return (status); + + for (i = 0; i < ci->children_num; i++) + { + oconfig_item_t *child = ci->children + i; + + if (strcasecmp ("Host", child->key) == 0) + status = cf_util_get_string (child, &host); + else if (strcasecmp ("Server", child->key) == 0) + status = cf_util_get_string (child, &server); + else if (strcasecmp ("Script", child->key) == 0) + status = cf_util_get_string (child, &script); + else + { + WARNING ("pinba plugin: Unknown config option: %s", child->key); + status = -1; + } + + if (status != 0) + break; } - - if ((*var = strdup (value)) == NULL) return (1); - else return (0); -} /* }}} int config_set */ + + if (status == 0) + service_statnode_add (name, host, server, script); + + sfree (name); + sfree (host); + sfree (server); + sfree (script); + + return (status); +} /* }}} int pinba_config_view */ static int plugin_config (oconfig_item_t *ci) /* {{{ */ { - unsigned int i, o; + int i; + /* The lock should not be necessary in the config callback, but let's be + * sure.. */ pthread_mutex_lock (&stat_nodes_lock); - /* FIXME XXX: Remove all those return statements that don't free the lock. */ - if (stat_nodes == NULL) + for (i = 0; i < ci->children_num; i++) { - /* Collect the "total" data by default. */ - service_statnode_add ("total", - /* host = */ NULL, - /* server = */ NULL, - /* script = */ NULL); - } - - for (i = 0; i < ci->children_num; i++) { oconfig_item_t *child = ci->children + i; - if (strcasecmp ("Address", child->key) == 0) { - if ((child->values_num != 1) || (child->values[0].type != OCONFIG_TYPE_STRING)){ - WARNING ("pinba plugin: `Address' needs exactly one string argument."); - return (-1); - } - config_set(&conf_node, child->values[0].value.string); - } else if (strcasecmp ("Port", child->key) == 0) { - if ((child->values_num != 1) || (child->values[0].type != OCONFIG_TYPE_STRING)){ - WARNING ("pinba plugin: `Port' needs exactly one string argument."); - return (-1); - } - config_set(&conf_service, child->values[0].value.string); - } else if (strcasecmp ("View", child->key) == 0) { - const char *name=NULL, *host=NULL, *server=NULL, *script=NULL; - if ((child->values_num != 1) || (child->values[0].type != OCONFIG_TYPE_STRING) || strlen(child->values[0].value.string)==0){ - WARNING ("pinba plugin: `View' needs exactly one non-empty string argument."); - return (-1); - } - name = child->values[0].value.string; - for(o=0; ochildren_num; o++){ - oconfig_item_t *node = child->children + o; - if (strcasecmp ("Host", node->key) == 0) { - if ((node->values_num != 1) || (node->values[0].type != OCONFIG_TYPE_STRING) || strlen(node->values[0].value.string)==0){ - WARNING ("pinba plugin: `View->Host' needs exactly one non-empty string argument."); - return (-1); - } - host = node->values[0].value.string; - } else if (strcasecmp ("Server", node->key) == 0) { - if ((node->values_num != 1) || (node->values[0].type != OCONFIG_TYPE_STRING) || strlen(node->values[0].value.string)==0){ - WARNING ("pinba plugin: `View->Server' needs exactly one non-empty string argument."); - return (-1); - } - server = node->values[0].value.string; - } else if (strcasecmp ("Script", node->key) == 0) { - if ((node->values_num != 1) || (node->values[0].type != OCONFIG_TYPE_STRING) || strlen(node->values[0].value.string)==0){ - WARNING ("pinba plugin: `View->Script' needs exactly one non-empty string argument."); - return (-1); - } - script = node->values[0].value.string; - } else { - WARNING ("pinba plugin: In `' context allowed only `Host', `Server' and `Script' options but not the `%s'.", node->key); - return (-1); - } - } - /* add new statnode */ - service_statnode_add(name, host, server, script); - } else { - WARNING ("pinba plugin: In `' context allowed only `Address', `Port' and `Observe' options but not the `%s'.", child->key); - return (-1); - } + + if (strcasecmp ("Address", child->key) == 0) + cf_util_get_string (child, &conf_node); + else if (strcasecmp ("Port", child->key) == 0) + cf_util_get_string (child, &conf_service); + else if (strcasecmp ("View", child->key) == 0) + pinba_config_view (child); + else + WARNING ("pinba plugin: Unknown config option: %s", child->key); } - + pthread_mutex_unlock(&stat_nodes_lock); return (0); @@ -651,6 +634,15 @@ static int plugin_init (void) /* {{{ */ { int status; + if (stat_nodes == NULL) + { + /* Collect the "total" data by default. */ + service_statnode_add ("total", + /* host = */ NULL, + /* server = */ NULL, + /* script = */ NULL); + } + if (collector_thread_running) return (0); -- 2.30.2