[collectd] [PATCH] utils_cmd_flush.c: Simplified / improved option parsing.

[Sebastian Harl]

Instead of parsing all options before doing any work, each option is now
handled right after it has been parsed. This has the following benefits:

 * No need to allocate and construct any data structures.
 * The timeout option may be used multiple times now, only affecting any
   plugins listed after this.

Signed-off-by: Sebastian Harl <sh at tokkee.org>
---
 src/collectd-unixsock.pod |    4 +-
 src/utils_cmd_flush.c     |  111 +++++++++++++-------------------------------
 2 files changed, 36 insertions(+), 79 deletions(-)

diff --git a/src/collectd-unixsock.pod b/src/collectd-unixsock.pod
index 3e25c76..53f8a57 100644
--- a/src/collectd-unixsock.pod
+++ b/src/collectd-unixsock.pod
@@ -180,7 +180,9 @@ Example:
 =item B<FLUSH> [B<timeout=>I<Timeout>] [B<plugin=>I<Plugin> [...]]
 
 Flushes all cached data older than I<Timeout> seconds. If no timeout has been
-specified, it defaults to -1 which causes all data to be flushed.
+specified, it defaults to -1 which causes all data to be flushed. B<timeout>
+may be specified multiple times - each occurrence applies to plugins listed
+afterwards.
 
 If specified, only specific plugins are flushed. Otherwise all plugins
 providing a flush callback are flushed.
diff --git a/src/utils_cmd_flush.c b/src/utils_cmd_flush.c
index c214d07..b1973be 100644
--- a/src/utils_cmd_flush.c
+++ b/src/utils_cmd_flush.c
@@ -25,109 +25,64 @@
 #include "common.h"
 #include "plugin.h"
 
-struct flush_info_s
+int handle_flush (FILE *fh, char **fields, int fields_num)
 {
-	char **plugins;
-	int plugins_num;
-	int timeout;
-};
-typedef struct flush_info_s flush_info_t;
+	int success = 0;
+	int error   = 0;
 
-static int parse_option_plugin (flush_info_t *fi, const char *option)
-{
-	char **temp;
+	int timeout = -1;
 
-	temp = (char **) realloc (fi->plugins,
-			(fi->plugins_num + 1) * sizeof (char *));
-	if (temp == NULL)
-	{
-		ERROR ("utils_cmd_flush: parse_option_plugin: realloc failed.");
-		return (-1);
-	}
-	fi->plugins = temp;
+	int i;
 
-	fi->plugins[fi->plugins_num] = strdup (option + strlen ("plugin="));
-	if (fi->plugins[fi->plugins_num] == NULL)
+	for (i = 1; i < fields_num; i++)
 	{
-		/* fi->plugins is freed in handle_flush in this case */
-		ERROR ("utils_cmd_flush: parse_option_plugin: strdup failed.");
-		return (-1);
-	}
-	fi->plugins_num++;
+		char *option = fields[i];
+		int   status = 0;
 
-	return (0);
-} /* int parse_option_plugin */
-
-static int parse_option_timeout (flush_info_t *fi, const char *option)
-{
-	const char *value_ptr = option + strlen ("timeout=");
-	char *endptr = NULL;
-	int timeout;
-
-	timeout = strtol (value_ptr, &endptr, 0);
-	if (value_ptr == endptr)
-		return (-1);
-
-	fi->timeout = (timeout <= 0) ? (-1) : timeout;
-
-	return (0);
-} /* int parse_option_timeout */
+		if (strncasecmp ("plugin=", option, strlen ("plugin=")) == 0)
+		{
+			char *plugin = option + strlen ("plugin=");
 
-static int parse_option (flush_info_t *fi, const char *option)
-{
-	if (strncasecmp ("plugin=", option, strlen ("plugin=")) == 0)
-		return (parse_option_plugin (fi, option));
-	else if (strncasecmp ("timeout=", option, strlen ("timeout=")) == 0)
-		return (parse_option_timeout (fi, option));
-	else
-		return (-1);
-} /* int parse_option */
+			if (0 == plugin_flush_one (timeout, plugin))
+				++success;
+			else
+				++error;
+		}
+		else if (strncasecmp ("timeout=", option, strlen ("timeout=")) == 0)
+		{
+			char *endptr = NULL;
+			char *value  = option + strlen ("timeout=");
 
-int handle_flush (FILE *fh, char **fields, int fields_num)
-{
-	flush_info_t fi;
-	int status;
-	int i;
+			errno = 0;
+			timeout = strtol (value, &endptr, 0);
 
-	memset (&fi, '\0', sizeof (fi));
-	fi.timeout = -1;
+			if ((endptr == value) || (0 != errno))
+				status = -1;
+			else if (0 >= timeout)
+				timeout = -1;
+		}
+		else
+			status = -1;
 
-	for (i = 1; i < fields_num; i++)
-	{
-		status = parse_option (&fi, fields[i]);
 		if (status != 0)
 		{
-			fprintf (fh, "-1 Cannot parse option %s\n", fields[i]);
+			fprintf (fh, "-1 Cannot parse option %s\n", option);
 			fflush (fh);
 			return (-1);
 		}
 	}
 
-	if (fi.plugins_num > 0)
+	if ((success + error) > 0)
 	{
-		int success = 0;
-		for (i = 0; i < fi.plugins_num; i++)
-		{
-			status = plugin_flush_one (fi.timeout, fi.plugins[i]);
-			if (status == 0)
-				success++;
-		}
-		fprintf (fh, "0 Done: %i successful, %i errors\n",
-				success, fi.plugins_num - success);
+		fprintf (fh, "0 Done: %i successful, %i errors\n", success, error);
 	}
 	else
 	{
-		plugin_flush_all (fi.timeout);
+		plugin_flush_all (timeout);
 		fprintf (fh, "0 Done\n");
 	}
 	fflush (fh);
 
-	for (i = 0; i < fi.plugins_num; i++)
-	{
-		sfree (fi.plugins[i]);
-	}
-	sfree (fi.plugins);
-
 	return (0);
 } /* int handle_flush */
 
-- 
1.5.5.rc1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20080325/fc080a8e/attachment.pgp