daemon: Ignore SIGPIPE.

The default action for the PIPE signal is to terminate the process. This
is not really what we want for collectd, as e.g. a client of the unixsock
plugin (which might even be running without root privileges) could kill
the daemon by closing the socket right after sending a request.

The signal now gets ignored and each I/O function is checked for success.
To simply that, the unixsock's output stream is now configured to be line
buffered, removing the need to call fflush() (which could fail as well and
would have to be checked for success).

While I was at it, I renamed the sigaction struct for SIGCHLD to fit the
coding style used elsewhere in collectd.

Signed-off-by: Sebastian Harl <sh@tokkee.org>
Signed-off-by: Florian Forster <octo@huhu.verplant.org>

diff --git a/src/collectd.c b/src/collectd.c
index f556fc5..a3f63b4 100644 (file)
--- a/src/collectd.c
+++ b/src/collectd.c
@@ -397,11 +397,12 @@ int main (int argc, char **argv)
        struct sigaction sig_int_action;
        struct sigaction sig_term_action;
        struct sigaction sig_usr1_action;
+       struct sigaction sig_pipe_action;
        char *configfile = CONFIGFILE;
        int test_config  = 0;
        const char *basedir;
 #if COLLECT_DAEMON
-       struct sigaction sigChldAction;
+       struct sigaction sig_chld_action;
        pid_t pid;
        int daemonize    = 1;
 #endif
@@ -486,9 +487,9 @@ int main (int argc, char **argv)
        /*
         * fork off child
         */
-       memset (&sigChldAction, '\0', sizeof (sigChldAction));
-       sigChldAction.sa_handler = SIG_IGN;
-       sigaction (SIGCHLD, &sigChldAction, NULL);
+       memset (&sig_chld_action, '\0', sizeof (sig_chld_action));
+       sig_chld_action.sa_handler = SIG_IGN;
+       sigaction (SIGCHLD, &sig_chld_action, NULL);
 
        if (daemonize)
        {
@@ -538,6 +539,10 @@ int main (int argc, char **argv)
        } /* if (daemonize) */
 #endif /* COLLECT_DAEMON */
 
+       memset (&sig_pipe_action, '\0', sizeof (sig_pipe_action));
+       sig_pipe_action.sa_handler = SIG_IGN;
+       sigaction (SIGPIPE, &sig_pipe_action, NULL);
+
        /*
         * install signal handlers
         */

diff --git a/src/unixsock.c b/src/unixsock.c
index 025c91d..d80091b 100644 (file)
--- a/src/unixsock.c
+++ b/src/unixsock.c
@@ -189,10 +189,34 @@ static void *us_handle_client (void *arg)
                pthread_exit ((void *) 1);
        }
 
-       while (fgets (buffer, sizeof (buffer), fhin) != NULL)
+       /* change output buffer to line buffered mode */
+       if (setvbuf (fhout, NULL, _IOLBF, 0) != 0)
+       {
+               char errbuf[1024];
+               ERROR ("unixsock plugin: setvbuf failed: %s",
+                               sstrerror (errno, errbuf, sizeof (errbuf)));
+               fclose (fhin);
+               fclose (fhout);
+               pthread_exit ((void *) 1);
+       }
+
+       while (42)
        {
                int len;
 
+               errno = 0;
+               if (fgets (buffer, sizeof (buffer), fhin) == NULL)
+               {
+                       if (errno != 0)
+                       {
+                               char errbuf[1024];
+                               WARNING ("unixsock plugin: failed to read from socket #%i: %s",
+                                               fileno (fhin),
+                                               sstrerror (errno, errbuf, sizeof (errbuf)));
+                       }
+                       break;
+               }
+
                len = strlen (buffer);
                while ((len > 0)
                                && ((buffer[len - 1] == '\n') || (buffer[len - 1] == '\r')))
@@ -234,8 +258,14 @@ static void *us_handle_client (void *arg)
                }
                else
                {
-                       fprintf (fhout, "-1 Unknown command: %s\n", fields[0]);
-                       fflush (fhout);
+                       if (fprintf (fhout, "-1 Unknown command: %s\n", fields[0]) < 0)
+                       {
+                               char errbuf[1024];
+                               WARNING ("unixsock plugin: failed to write to socket #%i: %s",
+                                               fileno (fhout),
+                                               sstrerror (errno, errbuf, sizeof (errbuf)));
+                               break;
+                       }
                }
        } /* while (fgets) */
 

diff --git a/src/utils_cmd_flush.c b/src/utils_cmd_flush.c
index b1973be..6fa8b7b 100644 (file)
--- a/src/utils_cmd_flush.c
+++ b/src/utils_cmd_flush.c
@@ -25,6 +25,14 @@
 #include "common.h"
 #include "plugin.h"
 
+#define print_to_socket(fh, ...) \
+       if (fprintf (fh, __VA_ARGS__) < 0) { \
+               char errbuf[1024]; \
+               WARNING ("handle_flush: failed to write to socket #%i: %s", \
+                               fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \
+               return -1; \
+       }
+
 int handle_flush (FILE *fh, char **fields, int fields_num)
 {
        int success = 0;
@@ -66,22 +74,21 @@ int handle_flush (FILE *fh, char **fields, int fields_num)
 
                if (status != 0)
                {
-                       fprintf (fh, "-1 Cannot parse option %s\n", option);
-                       fflush (fh);
+                       print_to_socket (fh, "-1 Cannot parse option %s\n", option);
                        return (-1);
                }
        }
 
        if ((success + error) > 0)
        {
-               fprintf (fh, "0 Done: %i successful, %i errors\n", success, error);
+               print_to_socket (fh, "0 Done: %i successful, %i errors\n",
+                               success, error);
        }
        else
        {
                plugin_flush_all (timeout);
-               fprintf (fh, "0 Done\n");
+               print_to_socket (fh, "0 Done\n");
        }
-       fflush (fh);
 
        return (0);
 } /* int handle_flush */

diff --git a/src/utils_cmd_getval.c b/src/utils_cmd_getval.c
index f110196..01c6899 100644 (file)
--- a/src/utils_cmd_getval.c
+++ b/src/utils_cmd_getval.c
@@ -25,6 +25,14 @@
 
 #include "utils_cache.h"
 
+#define print_to_socket(fh, ...) \
+  if (fprintf (fh, __VA_ARGS__) < 0) { \
+    char errbuf[1024]; \
+    WARNING ("handle_getval: failed to write to socket #%i: %s", \
+       fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \
+    return -1; \
+  }
+
 int handle_getval (FILE *fh, char **fields, int fields_num)
 {
   char *hostname;
@@ -45,17 +53,15 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
   if (fields_num != 2)
   {
     DEBUG ("unixsock plugin: Wrong number of fields: %i", fields_num);
-    fprintf (fh, "-1 Wrong number of fields: Got %i, expected 2.\n",
+    print_to_socket (fh, "-1 Wrong number of fields: Got %i, expected 2.\n",
        fields_num);
-    fflush (fh);
     return (-1);
   }
   DEBUG ("unixsock plugin: Got query for `%s'", fields[1]);
 
   if (strlen (fields[1]) < strlen ("h/p/t"))
   {
-    fprintf (fh, "-1 Invalied identifier, %s\n", fields[1]);
-    fflush (fh);
+    print_to_socket (fh, "-1 Invalied identifier, %s\n", fields[1]);
     return (-1);
   }
 
@@ -69,8 +75,7 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
   if (status != 0)
   {
     DEBUG ("unixsock plugin: Cannot parse `%s'", fields[1]);
-    fprintf (fh, "-1 Cannot parse identifier.\n");
-    fflush (fh);
+    print_to_socket (fh, "-1 Cannot parse identifier.\n");
     sfree (identifier_copy);
     return (-1);
   }
@@ -79,8 +84,7 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
   if (ds == NULL)
   {
     DEBUG ("unixsock plugin: plugin_get_ds (%s) == NULL;", type);
-    fprintf (fh, "-1 Type `%s' is unknown.\n", type);
-    fflush (fh);
+    print_to_socket (fh, "-1 Type `%s' is unknown.\n", type);
     sfree (identifier_copy);
     return (-1);
   }
@@ -90,8 +94,7 @@ 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\n");
-    fflush (fh);
+    print_to_socket (fh, "-1 No such value\n");
     sfree (identifier_copy);
     return (-1);
   }
@@ -101,24 +104,26 @@ int handle_getval (FILE *fh, char **fields, int fields_num)
     ERROR ("ds[%s]->ds_num = %i, "
        "but uc_get_rate_by_name returned %i values.",
        ds->type, ds->ds_num, values_num);
-    fprintf (fh, "-1 Error reading value from cache.\n");
-    fflush (fh);
+    print_to_socket (fh, "-1 Error reading value from cache.\n");
     sfree (values);
     sfree (identifier_copy);
     return (-1);
   }
 
-  fprintf (fh, "%u Value%s found\n", (unsigned int) values_num,
+  print_to_socket (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);
+    print_to_socket (fh, "%s=", ds->ds[i].name);
     if (isnan (values[i]))
-      fprintf (fh, "NaN\n");
+    {
+      print_to_socket (fh, "NaN\n");
+    }
     else
-      fprintf (fh, "%12e\n", values[i]);
+    {
+      print_to_socket (fh, "%12e\n", values[i]);
+    }
   }
-  fflush (fh);
 
   sfree (values);
   sfree (identifier_copy);

diff --git a/src/utils_cmd_listval.c b/src/utils_cmd_listval.c
index 8d6c783..6f03e75 100644 (file)
--- a/src/utils_cmd_listval.c
+++ b/src/utils_cmd_listval.c
@@ -26,6 +26,14 @@
 #include "utils_cmd_listval.h"
 #include "utils_cache.h"
 
+#define print_to_socket(fh, ...) \
+  if (fprintf (fh, __VA_ARGS__) < 0) { \
+    char errbuf[1024]; \
+    WARNING ("handle_listval: failed to write to socket #%i: %s", \
+       fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \
+    return -1; \
+  }
+
 int handle_listval (FILE *fh, char **fields, int fields_num)
 {
   char **names = NULL;
@@ -38,9 +46,8 @@ int handle_listval (FILE *fh, char **fields, int fields_num)
   {
     DEBUG ("command listval: us_handle_listval: Wrong number of fields: %i",
        fields_num);
-    fprintf (fh, "-1 Wrong number of fields: Got %i, expected 1.\n",
+    print_to_socket (fh, "-1 Wrong number of fields: Got %i, expected 1.\n",
        fields_num);
-    fflush (fh);
     return (-1);
   }
 
@@ -48,15 +55,14 @@ int handle_listval (FILE *fh, char **fields, int fields_num)
   if (status != 0)
   {
     DEBUG ("command listval: uc_get_names failed with status %i", status);
-    fprintf (fh, "-1 uc_get_names failed.\n");
-    fflush (fh);
+    print_to_socket (fh, "-1 uc_get_names failed.\n");
     return (-1);
   }
 
-  fprintf (fh, "%i Value%s found\n", (int) number, (number == 1) ? "" : "s");
+  print_to_socket (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);
+    print_to_socket (fh, "%u %s\n", (unsigned int) times[i], names[i]);
 
   return (0);
 } /* int handle_listval */

diff --git a/src/utils_cmd_putnotif.c b/src/utils_cmd_putnotif.c
index 18c1ece..eb7d60b 100644 (file)
--- a/src/utils_cmd_putnotif.c
+++ b/src/utils_cmd_putnotif.c
@@ -23,6 +23,14 @@
 #include "common.h"
 #include "plugin.h"
 
+#define print_to_socket(fh, ...) \
+  if (fprintf (fh, __VA_ARGS__) < 0) { \
+    char errbuf[1024]; \
+    WARNING ("handle_putnotif: failed to write to socket #%i: %s", \
+       fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \
+    return -1; \
+  }
+
 static int parse_option_severity (notification_t *n, char *value)
 {
   if (strcasecmp (value, "Failure") == 0)
@@ -107,9 +115,9 @@ int handle_putnotif (FILE *fh, char **fields, int fields_num)
   if (fields_num < 4)
   {
     DEBUG ("cmd putnotif: Wrong number of fields: %i", fields_num);
-    fprintf (fh, "-1 Wrong number of fields: Got %i, expected at least 4.\n",
+    print_to_socket (fh, "-1 Wrong number of fields: Got %i, "
+       "expected at least 4.\n",
        fields_num);
-    fflush (fh);
     return (-1);
   }
 
@@ -123,7 +131,7 @@ int handle_putnotif (FILE *fh, char **fields, int fields_num)
       status = parse_message (&n, fields + i, fields_num - i);
       if (status != 0)
       {
-       fprintf (fh, "-1 Error parsing the message. Have you hit the "
+       print_to_socket (fh, "-1 Error parsing the message. Have you hit the "
            "limit of %u bytes?\n", (unsigned int) sizeof (n.message));
       }
       break;
@@ -133,7 +141,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'\n", fields[i]);
+       print_to_socket (fh, "-1 Error parsing option `%s'\n", fields[i]);
        break;
       }
     }
@@ -142,17 +150,17 @@ int handle_putnotif (FILE *fh, char **fields, int fields_num)
   /* Check for required fields and complain if anything is missing. */
   if ((status == 0) && (n.severity == 0))
   {
-    fprintf (fh, "-1 Option `severity' missing.\n");
+    print_to_socket (fh, "-1 Option `severity' missing.\n");
     status = -1;
   }
   if ((status == 0) && (n.time == 0))
   {
-    fprintf (fh, "-1 Option `time' missing.\n");
+    print_to_socket (fh, "-1 Option `time' missing.\n");
     status = -1;
   }
   if ((status == 0) && (strlen (n.message) == 0))
   {
-    fprintf (fh, "-1 No message or message of length 0 given.\n");
+    print_to_socket (fh, "-1 No message or message of length 0 given.\n");
     status = -1;
   }
 
@@ -161,9 +169,8 @@ int handle_putnotif (FILE *fh, char **fields, int fields_num)
   if (status == 0)
   {
     plugin_dispatch_notification (&n);
-    fprintf (fh, "0 Success\n");
+    print_to_socket (fh, "0 Success\n");
   }
-  fflush (fh);
 
   return (0);
 } /* int handle_putnotif */

diff --git a/src/utils_cmd_putval.c b/src/utils_cmd_putval.c
index 98b5043..7552388 100644 (file)
--- a/src/utils_cmd_putval.c
+++ b/src/utils_cmd_putval.c
@@ -23,6 +23,14 @@
 #include "common.h"
 #include "plugin.h"
 
+#define print_to_socket(fh, ...) \
+       if (fprintf (fh, __VA_ARGS__) < 0) { \
+               char errbuf[1024]; \
+               WARNING ("handle_putval: failed to write to socket #%i: %s", \
+                               fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \
+               return -1; \
+       }
+
 static int parse_value (const data_set_t *ds, value_list_t *vl,
                const char *type,
                FILE *fh, char *buffer)
@@ -36,7 +44,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.\n");
+               print_to_socket (fh, "-1 No time found.\n");
                return (-1);
        }
        *value_str = '\0'; value_str++;
@@ -76,7 +84,7 @@ static int parse_value (const data_set_t *ds, value_list_t *vl,
                                "Number of values incorrect: "
                                "Got %i, expected %i. Identifier is `%s'.",
                                i, vl->values_len, identifier);
-               fprintf (fh, "-1 Number of values incorrect: "
+               print_to_socket (fh, "-1 Number of values incorrect: "
                                "Got %i, expected %i.\n",
                                i, vl->values_len);
                return (-1);
@@ -130,10 +138,9 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
        {
                DEBUG ("cmd putval: Wrong number of fields: %i",
                                fields_num);
-               fprintf (fh, "-1 Wrong number of fields: Got %i, "
+               print_to_socket (fh, "-1 Wrong number of fields: Got %i, "
                                "expected at least 3.\n",
                                fields_num);
-               fflush (fh);
                return (-1);
        }
 
@@ -147,8 +154,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
        if (status != 0)
        {
                DEBUG ("cmd putval: Cannot parse `%s'", fields[1]);
-               fprintf (fh, "-1 Cannot parse identifier.\n");
-               fflush (fh);
+               print_to_socket (fh, "-1 Cannot parse identifier.\n");
                sfree (identifier_copy);
                return (-1);
        }
@@ -160,8 +166,7 @@ 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.\n");
-               fflush (fh);
+               print_to_socket (fh, "-1 Identifier too long.\n");
                sfree (identifier_copy);
                return (-1);
        }
@@ -183,8 +188,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
        vl.values = (value_t *) malloc (vl.values_len * sizeof (value_t));
        if (vl.values == NULL)
        {
-               fprintf (fh, "-1 malloc failed.\n");
-               fflush (fh);
+               print_to_socket (fh, "-1 malloc failed.\n");
                sfree (identifier_copy);
                return (-1);
        }
@@ -204,7 +208,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'\n",
+                               print_to_socket (fh, "-1 Error parsing option `%s'\n",
                                                fields[i]);
                                break;
                        }
@@ -220,8 +224,7 @@ int handle_putval (FILE *fh, char **fields, int fields_num)
        /* Done parsing the options. */
 
        if (i == fields_num)
-               fprintf (fh, "0 Success\n");
-       fflush (fh);
+               print_to_socket (fh, "0 Success\n");
 
        sfree (vl.values); 
        sfree (identifier_copy);