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 f556fc5fd43fdd28647b58e4a71b563929858cbb..a3f63b48007336bc48621fa366cbf90a96ccff4c 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 025c91d5a6be76f6a2a94812af2f018782b17d43..d80091b1bd18bfffd290ae4c7a54b6086131605f 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 b1973be55bcdcaa5ab863f9de6b6f6092d9e3434..6fa8b7bf00b5d0b68979b4e828e1f7093e32f5dc 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 f110196acaf2c81c931f61cda0b01fe6a05660d3..01c68996a23f3abda83ac350e48989650180c5fb 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 8d6c7836fe565242eb155e8eb598071f6c97471a..6f03e757ac57b5c5d397c12821d7e6d2f45ee8c8 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 18c1eceedff4f834d5dc196f906bc2426ce18b34..eb7d60bc88657dfb5df94fec66940482c3d7b5c3 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 98b5043f33d1a94156fa509404a840af76dc69bb..755238815f57c15a4e63f9c0a830b8cb1abf05b3 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);