Merge pull request #178 from acv/refactor-wdctl-thread

Improve the wdctl_thread code

Author: mhaas

src/wdctl_thread.c

@@ -55,6 +55,7 @@
 #include "safe.h"
 
 
+static int create_unix_socket(const char *);
 static int write_to_socket(int, char *, size_t);
 static void *thread_wdctl_handler(void *);
 static void wdctl_status(int);
@@ -64,59 +65,72 @@ static void wdctl_restart(int);
 
 static int wdctl_socket_server;
 
-/** Launches a thread that monitors the control socket for request
-@param arg Must contain a pointer to a string containing the Unix domain socket to open
-@todo This thread loops infinitely, need a watchdog to verify that it is still running?
-*/
-void
-thread_wdctl(void *arg)
+static int
+create_unix_socket(const char *sock_name)
 {
-    int *fd;
-    char *sock_name;
     struct sockaddr_un sa_un;
-    int result;
-    pthread_t tid;
-    socklen_t len;
-
-    debug(LOG_DEBUG, "Starting wdctl.");
+    int sock;
 
     memset(&sa_un, 0, sizeof(sa_un));
-    sock_name = (char *)arg;
-    debug(LOG_DEBUG, "Socket name: %s", sock_name);
 
     if (strlen(sock_name) > (sizeof(sa_un.sun_path) - 1)) {
         /* TODO: Die handler with logging.... */
         debug(LOG_ERR, "WDCTL socket name too long");
-        exit(1);
+        return -1;
     }
 
-    debug(LOG_DEBUG, "Creating socket");
-    wdctl_socket_server = socket(PF_UNIX, SOCK_STREAM, 0);
+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
 
-    if (wdctl_socket_server < 0) {
-        debug(LOG_DEBUG, "Could not get server socket: %s", strerror(errno));
-        termination_handler(0);
+    if (sock < 0) {
+        debug(LOG_DEBUG, "Could not get unix socket: %s", strerror(errno));
+        return -1;
     }
-    debug(LOG_DEBUG, "Got server socket %d", wdctl_socket_server);
+    debug(LOG_DEBUG, "Got unix socket %d", sock);
 
     /* If it exists, delete... Not the cleanest way to deal. */
     unlink(sock_name);
 
     debug(LOG_DEBUG, "Filling sockaddr_un");
-    strcpy(sa_un.sun_path, sock_name);  /* XXX No size check because we
-                                         * check a few lines before. */
+    strcpy(sa_un.sun_path, sock_name);
     sa_un.sun_family = AF_UNIX;
 
     debug(LOG_DEBUG, "Binding socket (%s) (%d)", sa_un.sun_path, strlen(sock_name));
 
     /* Which to use, AF_UNIX, PF_UNIX, AF_LOCAL, PF_LOCAL? */
-    if (bind(wdctl_socket_server, (struct sockaddr *)&sa_un, sizeof(struct sockaddr_un))) {
-        debug(LOG_ERR, "Could not bind control socket: %s", strerror(errno));
-        termination_handler(0);
+    if (bind(sock, (struct sockaddr *)&sa_un, sizeof(struct sockaddr_un))) {
+        debug(LOG_ERR, "Could not bind unix socket: %s", strerror(errno));
+        return -1;
     }
 
-    if (listen(wdctl_socket_server, 5)) {
+    if (listen(sock, 5)) {
         debug(LOG_ERR, "Could not listen on control socket: %s", strerror(errno));
+        return -1;
+    }
+    return sock;
+}
+
+/** Launches a thread that monitors the control socket for request
+@param arg Must contain a pointer to a string containing the Unix domain socket to open
+@todo This thread loops infinitely, need a watchdog to verify that it is still running?
+*/
+void
+thread_wdctl(void *arg)
+{
+    int *fd;
+    char *sock_name;
+    struct sockaddr_un sa_un;
+    int result;
+    pthread_t tid;
+    socklen_t len;
+
+    debug(LOG_DEBUG, "Starting wdctl.");
+
+    sock_name = (char *)arg;
+    debug(LOG_DEBUG, "Socket name: %s", sock_name);
+
+    debug(LOG_DEBUG, "Creating socket");
+    wdctl_socket_server = create_unix_socket(sock_name);
+    if (-1 == wdctl_socket_server) {
         termination_handler(0);
     }
 
@@ -251,9 +265,9 @@ wdctl_restart(int afd)
 {
     int sock, fd;
     char *sock_name;
-    struct sockaddr_un sa_un;
     s_config *conf = NULL;
-    t_client *client = NULL;
+    struct sockaddr_un sa_un;
+    t_client *client;
     char *tempstring = NULL;
     pid_t pid;
     socklen_t len;
@@ -262,47 +276,13 @@ wdctl_restart(int afd)
 
     debug(LOG_NOTICE, "Will restart myself");
 
-    /*
-     * First, prepare the internal socket
-     */
-    memset(&sa_un, 0, sizeof(sa_un));
+    /* First, prepare the internal socket */
     sock_name = conf->internal_sock;
     debug(LOG_DEBUG, "Socket name: %s", sock_name);
 
-    if (strlen(sock_name) > (sizeof(sa_un.sun_path) - 1)) {
-        /* TODO: Die handler with logging.... */
-        debug(LOG_ERR, "INTERNAL socket name too long");
-        return;
-    }
-
     debug(LOG_DEBUG, "Creating socket");
-    sock = socket(PF_UNIX, SOCK_STREAM, 0);
-
-    if (sock < 0) {
-        debug(LOG_DEBUG, "Could not get server socket: %s", strerror(errno));
-        return;
-    }
-    debug(LOG_DEBUG, "Got internal socket %d", sock);
-
-    /* If it exists, delete... Not the cleanest way to deal. */
-    unlink(sock_name);
-
-    debug(LOG_DEBUG, "Filling sockaddr_un");
-    strcpy(sa_un.sun_path, sock_name);  /* XXX No size check because we check a few lines before. */
-    sa_un.sun_family = AF_UNIX;
-
-    debug(LOG_DEBUG, "Binding socket (%s) (%d)", sa_un.sun_path, strlen(sock_name));
-
-    /* Which to use, AF_UNIX, PF_UNIX, AF_LOCAL, PF_LOCAL? */
-    if (bind(sock, (struct sockaddr *)&sa_un, strlen(sock_name) + sizeof(sa_un.sun_family))) {
-        debug(LOG_ERR, "Could not bind internal socket: %s", strerror(errno));
-        close(sock);
-        return;
-    }
-
-    if (listen(sock, 5)) {
-        debug(LOG_ERR, "Could not listen on internal socket: %s", strerror(errno));
-        close(sock);
+    sock = create_unix_socket(sock_name);
+    if (-1 == sock) {
         return;
     }