Improve error detection in the pidfile api.

src/suricata.c

@@ -372,7 +372,7 @@ void usage(const char *progname)
     printf("\t--list-unittests             : list unit tests\n");
     printf("\t--fatal-unittests            : enable fatal failure on unittest error\n");
 #endif /* UNITTESTS */
-    printf("\t--pidfile                    : write pid to this file (only for daemon mode)\n");
+    printf("\t--pidfile <file>             : write pid to this file (only for daemon mode)\n");
     printf("\t--init-errors-fatal          : enable fatal failure on signature init error\n");
     printf("\t--dump-config                : show the running configuration\n");
     printf("\t--pfring-int <dev>           : run in pfring mode\n");
@@ -813,13 +813,18 @@ int main(int argc, char **argv)
     if (daemon == 1) {
         Daemonize();
         if (pid_filename != NULL) {
-            if (SCPidfileCreate(pid_filename) != 0)
+            if (SCPidfileCreate(pid_filename) != 0) {
                 pid_filename = NULL;
+                exit(EXIT_FAILURE);
+            }
         }
     } else {
-        if (pid_filename != NULL)
+        if (pid_filename != NULL) {
-            SCLogWarning(SC_ERR_PIDLOG, "The pidfile file option apply only to daemon modes");
+            SCLogError(SC_ERR_PIDFILE_DAEMON, "The pidfile file option applies "
-        pid_filename = NULL;
+                    "only to the daemon modes");
+            pid_filename = NULL;
+            exit(EXIT_FAILURE);
+        }
     }
 
 #ifndef OS_WIN32

src/util-error.c

@@ -18,7 +18,6 @@ const char * SCErrorToString(SCError err)
     switch (err) {
         CASE_CODE (SC_OK);
         CASE_CODE (SC_ERR_MEM_ALLOC);
-        CASE_CODE (SC_ERR_PIDLOG);
         CASE_CODE (SC_ERR_PCRE_MATCH);
         CASE_CODE (SC_ERR_PCRE_GET_SUBSTRING);
         CASE_CODE (SC_ERR_PCRE_COMPILE);
@@ -140,6 +139,10 @@ const char * SCErrorToString(SCError err)
         CASE_CODE (SC_ERR_LIBNET_NOT_ENABLED);
         CASE_CODE (SC_ERR_UNIFIED_LOG_FILE_HEADER);
         CASE_CODE (SC_ERR_REFERENCE_UNKNOWN);
+        CASE_CODE (SC_ERR_PIDFILE_SNPRINTF);
+        CASE_CODE (SC_ERR_PIDFILE_OPEN);
+        CASE_CODE (SC_ERR_PIDFILE_WRITE);
+        CASE_CODE (SC_ERR_PIDFILE_DAEMON);
 
         default:
             return "UNKNOWN_ERROR";

src/util-error.h

@@ -13,7 +13,6 @@
 typedef enum {
     SC_OK,
     SC_ERR_MEM_ALLOC,
-    SC_ERR_PIDLOG,
     SC_ERR_PCRE_MATCH,
     SC_ERR_PCRE_GET_SUBSTRING,
     SC_ERR_PCRE_COMPILE,
@@ -158,6 +157,10 @@ typedef enum {
                                           header writing function has been
                                           failed */
     SC_ERR_REFERENCE_UNKNOWN,       /**< unknown reference key (cve, url, etc) */
+    SC_ERR_PIDFILE_SNPRINTF,
+    SC_ERR_PIDFILE_OPEN,
+    SC_ERR_PIDFILE_WRITE,
+    SC_ERR_PIDFILE_DAEMON,
 
 } SCError;
 

src/util-pidfile.c

@@ -1,11 +1,11 @@
 /* Copyright (c) 2010 Open Infomation Security Foundation */
 
 /**
- * Copyright (c) 2009 Open Information Security Foundation
+ * \file
- *
  * \author Pablo Rincon Crespo <pablo.rincon.crespo@gmail.com>
  *         Adaptation of Steve Grubbs patch to our conding guidelines
  *         (thanks for the patch Steve ;)
+ * \author Victor Julien <victor@inliniac.net>
  */
 
 #include "suricata-common.h"
@@ -14,35 +14,56 @@
 /**
  * \brief Write a pid file (used at the startup)
  *        This commonly needed by the init scripts
+ *
  * \param pointer to the name of the pid file to write (optarg)
- * \retval 0 if succes; -1 on failure
+ *
+ * \retval 0 if succes
+ * \retval -1 on failure
  */
 int SCPidfileCreate(const char *pidfile)
 {
     SCEnter();
-    int pidfd, len;
+
+    int pidfd = 0;
     char val[16];
 
-    len = snprintf(val, sizeof(val), "%u\n", getpid());
+    size_t len = snprintf(val, sizeof(val), "%"PRIuMAX"\n", (uintmax_t)getpid());
     if (len <= 0) {
-        SCLogError(SC_ERR_PIDLOG, "Pid error (%s)", strerror(errno));
+        SCLogError(SC_ERR_PIDFILE_SNPRINTF, "Pid error (%s)", strerror(errno));
         SCReturnInt(-1);
     }
+
     pidfd = open(pidfile, O_CREAT | O_TRUNC | O_NOFOLLOW | O_WRONLY, 0644);
     if (pidfd < 0) {
-        SCLogError(SC_ERR_PIDLOG, "Unable to set pidfile (%s)", strerror(errno));
+        SCLogError(SC_ERR_PIDFILE_OPEN, "unable to set pidfile: %s", strerror(errno));
         SCReturnInt(-1);
     }
-    write(pidfd, val, (unsigned int)len);
+
+    ssize_t r = write(pidfd, val, (unsigned int)len);
+    if (r == -1) {
+        SCLogError(SC_ERR_PIDFILE_WRITE, "unable to write pidfile: %s", strerror(errno));
+        close(pidfd);
+        SCReturnInt(-1);
+    } else if ((size_t)r != len) {
+        SCLogError(SC_ERR_PIDFILE_WRITE, "unable to write pidfile: wrote"
+                " %"PRIdMAX" of %"PRIuMAX" bytes.", (intmax_t)r, (uintmax_t)len);
+        close(pidfd);
+        SCReturnInt(-1);
+    }
+
     close(pidfd);
     SCReturnInt(0);
 }
 
 /**
  * \brief Remove the pid file (used at the startup)
+ *
  * \param pointer to the name of the pid file to write (optarg)
  */
 void SCPidfileRemove(const char *pid_filename) {
-    if (pid_filename != NULL)
+    if (pid_filename != NULL) {
-        unlink(pid_filename);
+        /* we ignore the result, the user may have removed the file already. */
+        (void)unlink(pid_filename);
+    }
 }
+

src/util-pidfile.h

@@ -3,4 +3,6 @@
 
 int SCPidfileCreate(const char *);
 void SCPidfileRemove(const char *);
-#endif
+
+#endif /* __UTIL_PID_H__ */
+