From: Florian Forster <octo@huhu.verplant.org>
Date: Wed, 26 Mar 2008 08:56:41 +0000 (+0100)
Subject: Merge branch 'pull/master'
X-Git-Tag: collectd-4.4.0~66
X-Git-Url: https://git.octo.it/?a=commitdiff_plain;h=e8fc151794f5e3ca5c86259dbe201c41bae861d0;hp=ef493fef09db7227dcaedc2a3cae4a4a1ee4e1a9;p=collectd.git

Merge branch 'pull/master'
---

diff --git a/bindings/perl/Collectd/Unixsock.pm b/bindings/perl/Collectd/Unixsock.pm
index 3b6fa5d1..f21ebfe9 100644
--- a/bindings/perl/Collectd/Unixsock.pm
+++ b/bindings/perl/Collectd/Unixsock.pm
@@ -73,7 +73,7 @@ sub _create_socket
 
 =head1 VALUE IDENTIFIER
 
-The values in the collectd are identified using an five-tupel (host, plugin,
+The values in the collectd are identified using an five-tuple (host, plugin,
 plugin-instance, type, type-instance) where only plugin-instance and
 type-instance may be NULL (or undefined). Many functions expect an
 I<%identifier> hash that has at least the members B<host>, B<plugin>, and
@@ -217,7 +217,7 @@ sub getval
 =item I<$obj>-E<gt>B<putval> (I<%identifier>, B<time> =E<gt> I<$time>, B<values> =E<gt> [...]);
 
 Submits a value-list to the daemon. If the B<time> argument is omitted
-C<time()> is used. The requierd argument B<values> is a reference to an array
+C<time()> is used. The required argument B<values> is a reference to an array
 of values that is to be submitted. The number of values must match the number
 of values expected for the given B<type> (see L<VALUE IDENTIFIER>), though this
 is checked by the daemon, not the Perl module. Also, gauge data-sources
diff --git a/contrib/cussh.pl b/contrib/cussh.pl
index c19c3f30..6da2856d 100755
--- a/contrib/cussh.pl
+++ b/contrib/cussh.pl
@@ -1,7 +1,7 @@
 #!/usr/bin/perl
 #
 # collectd - contrib/cussh.pl
-# Copyright (C) 2007  Sebastian Harl
+# Copyright (C) 2007-2008  Sebastian Harl
 #
 # This program is free software; you can redistribute it and/or modify it
 # under the terms of the GNU General Public License as published by the
@@ -57,9 +57,10 @@ use Collectd::Unixsock();
 	my $sock = Collectd::Unixsock->new($path);
 
 	my $cmds = {
-		PUTVAL => \&putval,
-		GETVAL => \&getval,
-		FLUSH  => \&flush,
+		PUTVAL  => \&putval,
+		GETVAL  => \&getval,
+		FLUSH   => \&flush,
+		LISTVAL => \&listval,
 	};
 
 	if (! $sock) {
@@ -67,7 +68,7 @@ use Collectd::Unixsock();
 		exit 1;
 	}
 
-	print "cussh version 0.2, Copyright (C) 2007 Sebastian Harl\n"
+	print "cussh version 0.2, Copyright (C) 2007-2008 Sebastian Harl\n"
 		. "cussh comes with ABSOLUTELY NO WARRANTY. This is free software,\n"
 		. "and you are welcome to redistribute it under certain conditions.\n"
 		. "See the GNU General Public License 2 for more details.\n\n";
@@ -76,9 +77,13 @@ use Collectd::Unixsock();
 		print "cussh> ";
 		my $line = <STDIN>;
 
-		last if ((! $line) || ($line =~ m/^quit$/i));
+		last if (! $line);
 
-		my ($cmd) = $line =~ m/^(\w+)\s+/;
+		chomp $line;
+
+		last if ($line =~ m/^quit$/i);
+
+		my ($cmd) = $line =~ m/^(\w+)\s*/;
 		$line = $';
 
 		next if (! $cmd);
@@ -108,7 +113,7 @@ sub getid {
 
 	print $$string . $/;
 	my ($h, $p, $pi, $t, $ti) =
-		$$string =~ m/^(\w+)\/(\w+)(?:-(\w+))?\/(\w+)(?:-(\w+))?\s+/;
+		$$string =~ m/^(\w+)\/(\w+)(?:-(\w+))?\/(\w+)(?:-(\w+))?\s*/;
 	$$string = $';
 
 	return if ((! $h) || (! $p) || (! $t));
@@ -122,17 +127,31 @@ sub getid {
 	return \%id;
 }
 
+sub putid {
+	my $ident = shift || return;
+
+	my $string;
+
+	$string = $ident->{'host'} . "/" . $ident->{'plugin'};
+
+	if (defined $ident->{'plugin_instance'}) {
+		$string .= "-" . $ident->{'plugin_instance'};
+	}
+
+	$string .= "/" . $ident->{'type'};
+
+	if (defined $ident->{'type_instance'}) {
+		$string .= "-" . $ident->{'type_instance'};
+	}
+	return $string;
+}
+
 =head1 COMMANDS
 
 =over 4
 
 =item B<GETVAL> I<Identifier>
 
-=item B<PUTVAL> I<Identifier> I<Valuelist>
-
-These commands follow the exact same syntax as described in
-L<collectd-unixsock(5)>.
-
 =cut
 
 sub putval {
@@ -147,9 +166,13 @@ sub putval {
 	}
 
 	my ($time, @values) = split m/:/, $line;
-	return $sock->putval(%$id, $time, \@values);
+	return $sock->putval(%$id, time => $time, values => \@values);
 }
 
+=item B<PUTVAL> I<Identifier> I<Valuelist>
+
+=cut
+
 sub getval {
 	my $sock = shift || return;
 	my $line = shift || return;
@@ -163,7 +186,10 @@ sub getval {
 
 	my $vals = $sock->getval(%$id);
 
-	return if (! $vals);
+	if (! $vals) {
+		print STDERR $sock->{'error'} . $/;
+		return;
+	}
 
 	foreach my $key (keys %$vals) {
 		print "\t$key: $vals->{$key}\n";
@@ -171,6 +197,10 @@ sub getval {
 	return 1;
 }
 
+=item B<FLUSH> [B<timeout>=I<$timeout>] [B<plugin>=I<$plugin>[ ...]]
+
+=cut
+
 sub flush {
 	my $sock = shift || return;
 	my $line = shift;
@@ -209,6 +239,33 @@ sub flush {
 	return 1;
 }
 
+=item B<LISTVAL>
+
+=cut
+
+sub listval {
+	my $sock = shift || return;
+
+	my @res;
+
+	@res = $sock->listval();
+
+	if (! @res) {
+		print STDERR $sock->{'error'} . $/;
+		return;
+	}
+
+	foreach my $ident (@res) {
+		print $ident->{'time'} . " " . putid($ident) . $/;
+	}
+	return 1;
+}
+
+=back
+
+These commands follow the exact same syntax as described in
+L<collectd-unixsock(5)>.
+
 =head1 SEE ALSO
 
 L<collectd(1)>, L<collectd-unisock(5)>
diff --git a/src/collectd-unixsock.pod b/src/collectd-unixsock.pod
index 2ae5163e..971cb36d 100644
--- a/src/collectd-unixsock.pod
+++ b/src/collectd-unixsock.pod
@@ -29,6 +29,18 @@ Upon start the C<unixsock plugin> opens a UNIX-socket and waits for
 connections. Once a connection is established the client can send commands to
 the daemon which it will answer, if it understand them.
 
+In general the plugin answers with a status line of the following form:
+
+I<Status> I<Message>
+
+If I<Status> is greater than or equal to zero the message indicates success,
+if I<Status> is less than zero the message indicates failure. I<Message> is a
+human-readable string that further describes the return value.
+
+On success, I<Status> furthermore indicates the number of subsequent lines of
+output (not including the status line). Each such lines usually contains a
+single return value. See the description of each command for details.
+
 The following commands are implemented:
 
 =over 4
@@ -36,40 +48,34 @@ The following commands are implemented:
 =item B<GETVAL> I<Identifier>
 
 If the value identified by I<Identifier> (see below) is found the complete
-value-list is returned. The response is a space separated list of
-name-value-pairs:
-
-I<num> I<name>B<=>I<value>[ I<name>B<=>I<value>[ ...]]
-
-If I<num> is less then zero, an error occurred. Otherwise it contains the
-number of values that follow. Each value is of the form I<name>B<=>I<value>.
+value-list is returned. The response is a list of name-value-pairs, each pair
+on its own line (the number of lines is indicated by the status line - see
+above). Each name-value-pair is of the form I<name>B<=>I<value>.
 Counter-values are converted to a rate, e.E<nbsp>g. bytes per second.
 Undefined values are returned as B<NaN>.
 
 Example:
   -> | GETVAL myhost/cpu-0/cpu-user
-  <- | 1 value=1.260000e+00
+  <- | 1 Value found
+  <- | value=1.260000e+00
 
 =item B<LISTVAL>
 
 Returns a list of the values available in the value cache together with the
 time of the last update, so that querying applications can issue a B<GETVAL>
-command for the values that have changed.
-
-The first line's status number is the number of identifiers returned or less
-than zero if an error occurred. Each of the following lines contains the
+command for the values that have changed. Each return value consists of the
 update time as an epoch value and the identifier, separated by a space. The
 update time is the time of the last value, as provided by the collecting
-instance and may be very different from the time the server consideres to be
+instance and may be very different from the time the server considers to be
 "now".
 
 Example:
   -> | LISTVAL
   <- | 69 Values found
-  <- | 1182204284 leeloo/cpu-0/cpu-idle
-  <- | 1182204284 leeloo/cpu-0/cpu-nice
-  <- | 1182204284 leeloo/cpu-0/cpu-system
-  <- | 1182204284 leeloo/cpu-0/cpu-user
+  <- | 1182204284 myhost/cpu-0/cpu-idle
+  <- | 1182204284 myhost/cpu-0/cpu-nice
+  <- | 1182204284 myhost/cpu-0/cpu-system
+  <- | 1182204284 myhost/cpu-0/cpu-user
   ...
 
 =item B<PUTVAL> I<Identifier> [I<OptionList>] I<Valuelist>
@@ -180,7 +186,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.
@@ -206,26 +214,14 @@ some examples:
   myhost/memory/memory-used
   myhost/disk-sda/disk_octets
 
-=head2 Return values
-
-Unless otherwise noted the plugin answers with a line of the following form:
-
-I<Num> I<Message>
-
-If I<Num> is zero the message indicates success, if I<Num> is non-zero the
-message indicates failure. I<Message> is a human-readable string that describes
-the return value further.
-
-Commands that return values may use I<Num> to return the number of values that
-follow, such as the B<GETVAL> command. These commands usually return a negative
-value on failure and never return zero.
-
 =head1 ABSTRACTION LAYER
 
-Shipped with the sourcecode comes the Perl-Module L<Collectd::Unixsock> which
+B<collectd> ships the Perl-Module L<Collectd::Unixsock> which
 provides an abstraction layer over the actual socket connection. It can be
-found in the directory F<contrib/PerlLib>. If you want to use Perl to
-communicate with the daemon, you're encouraged to use and expand this module.
+found in the directory F<bindings/perl/> in the source distribution or
+(usually) somewhere near F</usr/share/perl5/> if you're using a package. If
+you want to use Perl to communicate with the daemon, you're encouraged to use
+and expand this module.
 
 =head1 SEE ALSO
 
diff --git a/src/unixsock.c b/src/unixsock.c
index 8701cb9d..80198ce2 100644
--- a/src/unixsock.c
+++ b/src/unixsock.c
@@ -158,7 +158,7 @@ static int us_open_socket (void)
 static void *us_handle_client (void *arg)
 {
 	int fd;
-	FILE *fh;
+	FILE *fhin, *fhout;
 	char buffer[1024];
 	char *fields[128];
 	int   fields_num;
@@ -169,8 +169,8 @@ static void *us_handle_client (void *arg)
 
 	DEBUG ("Reading from fd #%i", fd);
 
-	fh = fdopen (fd, "r+");
-	if (fh == NULL)
+	fhin  = fdopen (fd, "r");
+	if (fhin == NULL)
 	{
 		char errbuf[1024];
 		ERROR ("unixsock plugin: fdopen failed: %s",
@@ -179,7 +179,17 @@ static void *us_handle_client (void *arg)
 		pthread_exit ((void *) 1);
 	}
 
-	while (fgets (buffer, sizeof (buffer), fh) != NULL)
+	fhout = fdopen (fd, "w");
+	if (fhout == NULL)
+	{
+		char errbuf[1024];
+		ERROR ("unixsock plugin: fdopen failed: %s",
+				sstrerror (errno, errbuf, sizeof (errbuf)));
+		fclose (fhin); /* this closes fd as well */
+		pthread_exit ((void *) 1);
+	}
+
+	while (fgets (buffer, sizeof (buffer), fhin) != NULL)
 	{
 		int len;
 
@@ -204,33 +214,34 @@ static void *us_handle_client (void *arg)
 
 		if (strcasecmp (fields[0], "getval") == 0)
 		{
-			handle_getval (fh, fields, fields_num);
+			handle_getval (fhout, fields, fields_num);
 		}
 		else if (strcasecmp (fields[0], "putval") == 0)
 		{
-			handle_putval (fh, fields, fields_num);
+			handle_putval (fhout, fields, fields_num);
 		}
 		else if (strcasecmp (fields[0], "listval") == 0)
 		{
-			handle_listval (fh, fields, fields_num);
+			handle_listval (fhout, fields, fields_num);
 		}
 		else if (strcasecmp (fields[0], "putnotif") == 0)
 		{
-			handle_putnotif (fh, fields, fields_num);
+			handle_putnotif (fhout, fields, fields_num);
 		}
 		else if (strcasecmp (fields[0], "flush") == 0)
 		{
-			handle_flush (fh, fields, fields_num);
+			handle_flush (fhout, fields, fields_num);
 		}
 		else
 		{
-			fprintf (fh, "-1 Unknown command: %s\n", fields[0]);
-			fflush (fh);
+			fprintf (fhout, "-1 Unknown command: %s\n", fields[0]);
+			fflush (fhout);
 		}
 	} /* while (fgets) */
 
 	DEBUG ("Exiting..");
-	close (fd);
+	fclose (fhin);
+	fclose (fhout);
 
 	pthread_exit ((void *) 0);
 	return ((void *) 0);
diff --git a/src/utils_cmd_flush.c b/src/utils_cmd_flush.c
index c214d074..b1973be5 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 */
 
diff --git a/src/utils_cmd_getval.c b/src/utils_cmd_getval.c
index a4edf4f9..f110196a 100644
--- a/src/utils_cmd_getval.c
+++ b/src/utils_cmd_getval.c
@@ -35,6 +35,8 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
   gauge_t *values;
   size_t values_num;
 
+  char *identifier_copy;
+
   const data_set_t *ds;
 
   int   status;
@@ -52,12 +54,16 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
 
   if (strlen (fields[1]) < strlen ("h/p/t"))
   {
-    fprintf (fh, "-1 Invalied identifier, %s", fields[1]);
+    fprintf (fh, "-1 Invalied identifier, %s\n", fields[1]);
     fflush (fh);
     return (-1);
   }
 
-  status = parse_identifier (fields[1], &hostname,
+  /* parse_identifier() modifies its first argument,
+   * returning pointers into it */
+  identifier_copy = sstrdup (fields[1]);
+
+  status = parse_identifier (identifier_copy, &hostname,
       &plugin, &plugin_instance,
       &type, &type_instance);
   if (status != 0)
@@ -65,6 +71,7 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
     DEBUG ("unixsock plugin: Cannot parse `%s'", fields[1]);
     fprintf (fh, "-1 Cannot parse identifier.\n");
     fflush (fh);
+    sfree (identifier_copy);
     return (-1);
   }
 
@@ -74,6 +81,7 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
     DEBUG ("unixsock plugin: plugin_get_ds (%s) == NULL;", type);
     fprintf (fh, "-1 Type `%s' is unknown.\n", type);
     fflush (fh);
+    sfree (identifier_copy);
     return (-1);
   }
 
@@ -82,8 +90,9 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
   status = uc_get_rate_by_name (fields[1], &values, &values_num);
   if (status != 0)
   {
-    fprintf (fh, "-1 No such value");
+    fprintf (fh, "-1 No such value\n");
     fflush (fh);
+    sfree (identifier_copy);
     return (-1);
   }
 
@@ -95,23 +104,24 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
     fprintf (fh, "-1 Error reading value from cache.\n");
     fflush (fh);
     sfree (values);
+    sfree (identifier_copy);
     return (-1);
   }
 
-  fprintf (fh, "%u", (unsigned int) values_num);
+  fprintf (fh, "%u Value%s found\n", (unsigned int) values_num,
+      (values_num == 1) ? "" : "s");
   for (i = 0; i < values_num; i++)
   {
-    fprintf (fh, " %s=", ds->ds[i].name);
+    fprintf (fh, "%s=", ds->ds[i].name);
     if (isnan (values[i]))
-      fprintf (fh, "NaN");
+      fprintf (fh, "NaN\n");
     else
-      fprintf (fh, "%12e", values[i]);
+      fprintf (fh, "%12e\n", values[i]);
   }
-
-  fprintf (fh, "\n");
   fflush (fh);
 
   sfree (values);
+  sfree (identifier_copy);
 
   return (0);
 } /* int handle_getval */
diff --git a/src/utils_cmd_listval.c b/src/utils_cmd_listval.c
index 644a67cc..8d6c7836 100644
--- a/src/utils_cmd_listval.c
+++ b/src/utils_cmd_listval.c
@@ -53,7 +53,7 @@ int handle_listval (FILE *fh, char **fields, int fields_num)
     return (-1);
   }
 
-  fprintf (fh, "%i Values found\n", (int) number);
+  fprintf (fh, "%i Value%s found\n", (int) number, (number == 1) ? "" : "s");
   for (i = 0; i < number; i++)
     fprintf (fh, "%u %s\n", (unsigned int) times[i], names[i]);
   fflush (fh);
diff --git a/src/utils_cmd_putnotif.c b/src/utils_cmd_putnotif.c
index 0282e4e2..18c1ecee 100644
--- a/src/utils_cmd_putnotif.c
+++ b/src/utils_cmd_putnotif.c
@@ -133,7 +133,7 @@ int handle_putnotif (FILE *fh, char **fields, int fields_num)
       status = parse_option (&n, fields[i]);
       if (status != 0)
       {
-	fprintf (fh, "-1 Error parsing option `%s'", fields[i]);
+	fprintf (fh, "-1 Error parsing option `%s'\n", fields[i]);
 	break;
       }
     }
diff --git a/src/utils_cmd_putval.c b/src/utils_cmd_putval.c
index 7cf0b6da..98b5043f 100644
--- a/src/utils_cmd_putval.c
+++ b/src/utils_cmd_putval.c
@@ -36,7 +36,7 @@ static int parse_value (const data_set_t *ds, value_list_t *vl,
 	char *value_str = strchr (time_str, ':');
 	if (value_str == NULL)
 	{
-		fprintf (fh, "-1 No time found.");
+		fprintf (fh, "-1 No time found.\n");
 		return (-1);
 	}
 	*value_str = '\0'; value_str++;
@@ -121,6 +121,8 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 	int   status;
 	int   i;
 
+	char *identifier_copy;
+
 	const data_set_t *ds;
 	value_list_t vl = VALUE_LIST_INIT;
 
@@ -135,7 +137,11 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 		return (-1);
 	}
 
-	status = parse_identifier (fields[1], &hostname,
+	/* parse_identifier() modifies its first argument,
+	 * returning pointers into it */
+	identifier_copy = sstrdup (fields[1]);
+
+	status = parse_identifier (identifier_copy, &hostname,
 			&plugin, &plugin_instance,
 			&type, &type_instance);
 	if (status != 0)
@@ -143,6 +149,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 		DEBUG ("cmd putval: Cannot parse `%s'", fields[1]);
 		fprintf (fh, "-1 Cannot parse identifier.\n");
 		fflush (fh);
+		sfree (identifier_copy);
 		return (-1);
 	}
 
@@ -153,7 +160,9 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 			|| ((type_instance != NULL)
 				&& (strlen (type_instance) >= sizeof (vl.type_instance))))
 	{
-		fprintf (fh, "-1 Identifier too long.");
+		fprintf (fh, "-1 Identifier too long.\n");
+		fflush (fh);
+		sfree (identifier_copy);
 		return (-1);
 	}
 
@@ -165,14 +174,18 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 		strcpy (vl.type_instance, type_instance);
 
 	ds = plugin_get_ds (type);
-	if (ds == NULL)
+	if (ds == NULL) {
+		sfree (identifier_copy);
 		return (-1);
+	}
 
 	vl.values_len = ds->ds_num;
 	vl.values = (value_t *) malloc (vl.values_len * sizeof (value_t));
 	if (vl.values == NULL)
 	{
-		fprintf (fh, "-1 malloc failed.");
+		fprintf (fh, "-1 malloc failed.\n");
+		fflush (fh);
+		sfree (identifier_copy);
 		return (-1);
 	}
 
@@ -191,7 +204,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 		{
 			if (parse_option (&vl, fields[i]) != 0)
 			{
-				fprintf (fh, "-1 Error parsing option `%s'",
+				fprintf (fh, "-1 Error parsing option `%s'\n",
 						fields[i]);
 				break;
 			}
@@ -211,6 +224,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
 	fflush (fh);
 
 	sfree (vl.values); 
+	sfree (identifier_copy);
 
 	return (0);
 } /* int handle_putval */