Merge branch 'pull/master'
authorFlorian Forster <octo@huhu.verplant.org>
Wed, 26 Mar 2008 08:56:41 +0000 (09:56 +0100)
committerFlorian Forster <octo@huhu.verplant.org>
Wed, 26 Mar 2008 08:56:41 +0000 (09:56 +0100)
bindings/perl/Collectd/Unixsock.pm
contrib/cussh.pl
src/collectd-unixsock.pod
src/unixsock.c
src/utils_cmd_flush.c
src/utils_cmd_getval.c
src/utils_cmd_listval.c
src/utils_cmd_putnotif.c
src/utils_cmd_putval.c

index 3b6fa5d..f21ebfe 100644 (file)
@@ -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
index c19c3f3..6da2856 100755 (executable)
@@ -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)>
index 2ae5163..971cb36 100644 (file)
@@ -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
 
index 8701cb9..80198ce 100644 (file)
@@ -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);
index c214d07..b1973be 100644 (file)
 #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 */
 
index a4edf4f..f110196 100644 (file)
@@ -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 */
index 644a67c..8d6c783 100644 (file)
@@ -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);
index 0282e4e..18c1ece 100644 (file)
@@ -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;
       }
     }
index 7cf0b6d..98b5043 100644 (file)
@@ -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 */