Merge pull request #165 from acv/new-coverity-pass

mhaas · mhaas · commit ce6de4761bf3 · 2015-03-29T08:28:36.000+02:00
More coverity fixes
diff --git a/src/auth.c b/src/auth.c
@@ -48,11 +48,6 @@
 #include "client_list.h"
 #include "util.h"
 
-/* Defined in clientlist.c */
-extern	pthread_mutex_t	client_list_mutex;
-
-/* Defined in util.c */
-extern long served_this_session;
 
 /** Launches a thread that periodically checks if any of the connections has timed out
 @param arg Must contain a pointer to a string containing the IP adress of the client to check to check
diff --git a/src/centralserver.c b/src/centralserver.c
@@ -52,8 +52,6 @@
 
 #include "simple_http.h"
 
-extern pthread_mutex_t config_mutex;
-
 /** Initiates a transaction with the auth server, either to authenticate or to
  * update the traffic counters at the server
 @param authresponse Returns the information given by the central server 
diff --git a/src/client_list.c b/src/client_list.c
@@ -1,3 +1,4 @@
+/* vim: set et sw=4 ts=4 sts=4 : */
 /********************************************************************\
  * This program is free software; you can redistribute it and/or    *
  * modify it under the terms of the GNU General Public License as   *
@@ -18,9 +19,6 @@
  *                                                                  *
  \********************************************************************/
 
-/*
- * $Id$
- */
 /** @file client_list.c
   @brief Client List Functions
   @author Copyright (C) 2004 Alexandre Carmel-Veillex <acv@acv.ca>
@@ -44,15 +42,14 @@
 #include "conf.h"
 #include "client_list.h"
 
-/** Global mutex to protect access to the client list */
-extern pthread_mutex_t client_list_mutex;
-pthread_mutex_t client_list_mutex = PTHREAD_MUTEX_INITIALIZER;
-
 /** @internal
  * Holds a pointer to the first element of the list 
  */ 
 static t_client         *firstclient = NULL;
 
+/** Global mutex to protect access to the client list */
+pthread_mutex_t client_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 /** Get a new client struct, not added to the list yet
  * @return Pointer to newly created client object not on the list yet.
  */
diff --git a/src/client_list.h b/src/client_list.h
@@ -1,3 +1,4 @@
+/* vim: set et sw=4 ts=4 sts=4 : */
 /********************************************************************\
  * This program is free software; you can redistribute it and/or    *
  * modify it under the terms of the GNU General Public License as   *
@@ -27,6 +28,9 @@
 #ifndef _CLIENT_LIST_H_
 #define _CLIENT_LIST_H_
 
+/** Global mutex to protect access to the client list */
+extern pthread_mutex_t client_list_mutex;
+
 /** Counters struct for a client's bandwidth usage (in bytes)
  */
 typedef struct _t_counters {
diff --git a/src/commandline.c b/src/commandline.c
@@ -1,3 +1,4 @@
+/* vim: set et sw=4 ts=4 sts=4 : */
 /********************************************************************\
  * This program is free software; you can redistribute it and/or    *
  * modify it under the terms of the GNU General Public License as   *
@@ -18,7 +19,6 @@
  *                                                                  *
 \********************************************************************/
 
-/* $Id$ */
 /** @file commandline.c
     @brief Command line argument handling
     @author Copyright (C) 2004 Philippe April <papril777@yahoo.com>
@@ -33,28 +33,23 @@
 #include "debug.h"
 #include "safe.h"
 #include "conf.h"
+#include "commandline.h"
 
 #include "../config.h"
 
 /*
  * Holds an argv that could be passed to exec*() if we restart ourselves
  */
-/* Declare variable */
-extern char **restartargv;
-/* Define variable */
-char **restartargv = NULL;
+char ** restartargv = NULL;
 
-static void usage(void);
-
-void parse_commandline(int argc, char **argv);
-
-/*
+/**
  * A flag to denote whether we were restarted via a parent wifidog, or started normally
  * 0 means normally, otherwise it will be populated by the PID of the parent
  */
-extern pid_t restart_orig_pid;
 pid_t restart_orig_pid = 0;
 
+static void usage(void);
+
 /** @internal
  * @brief Print usage
  *
@@ -97,7 +92,6 @@ parse_commandline(int argc, char **argv)
     restartargv[i++] = safe_strdup(argv[0]);
 
     while (-1 != (c = getopt(argc, argv, "c:hfd:sw:vx:i:"))) {
-
         skiponrestart = 0;
 
         switch (c) {
@@ -185,5 +179,4 @@ parse_commandline(int argc, char **argv)
     restartargv[i++] = NULL;
     restartargv[i++] = NULL;
     restartargv[i++] = NULL;
-
 }
diff --git a/src/commandline.h b/src/commandline.h
@@ -1,3 +1,4 @@
+/* vim: set et sw=4 ts=4 sts=4 : */
 /********************************************************************\
  * This program is free software; you can redistribute it and/or    *
  * modify it under the terms of the GNU General Public License as   *
@@ -27,6 +28,17 @@
 #ifndef _COMMANDLINE_H_
 #define _COMMANDLINE_H_
 
+/*
+ * Holds an argv that could be passed to exec*() if we restart ourselves
+ */
+extern char **restartargv;
+
+/**
+ * A flag to denote whether we were restarted via a parent wifidog, or started normally
+ * 0 means normally, otherwise it will be populated by the PID of the parent
+ */
+extern pid_t restart_orig_pid;
+
 /** @brief Parses the command line and set the config accordingly */
 void parse_commandline(int, char **);
 
diff --git a/src/conf.c b/src/conf.c
@@ -55,7 +55,6 @@ static s_config config;
 /**
  * Mutex for the configuration file, used by the auth_servers related
  * functions. */
-extern pthread_mutex_t config_mutex;
 pthread_mutex_t config_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 /** @internal
diff --git a/src/conf.h b/src/conf.h
@@ -83,6 +83,11 @@
 #define FWRULESET_LOCKED_USERS "locked-users"
 /*@}*/
 
+/**
+ * Mutex for the configuration file, used by the auth_servers related
+ * functions. */
+extern pthread_mutex_t config_mutex;
+
 /**
  * Information about the authentication server
  */
diff --git a/src/firewall.c b/src/firewall.c
@@ -1,3 +1,4 @@
+/* vim: set et sw=4 ts=4 sts=4 : */
 /********************************************************************\
  * This program is free software; you can redistribute it and/or    *
  * modify it under the terms of the GNU General Public License as   *
@@ -18,9 +19,6 @@
  *                                                                  *
  \********************************************************************/
 
-/*
- * $Id$
- */
 /** @internal
   @file firewall.c
   @brief Firewall update functions
@@ -64,13 +62,7 @@
 #include "auth.h"
 #include "centralserver.h"
 #include "client_list.h"
-
-extern pthread_mutex_t client_list_mutex;
-
-/* from commandline.c */
-extern pid_t restart_orig_pid;
-
-
+#include "commandline.h"
 
 /**
  * Allow a client access through the firewall by adding a rule in the firewall to MARK the user's packets with the proper
diff --git a/src/fw_iptables.c b/src/fw_iptables.c
@@ -52,9 +52,6 @@ static int iptables_do_command(const char *format, ...);
 static char *iptables_compile(const char *, const char *, const t_firewall_rule *);
 static void iptables_load_ruleset(const char *, const char *, const char *);
 
-extern pthread_mutex_t	client_list_mutex;
-extern pthread_mutex_t	config_mutex;
-
 /**
 Used to supress the error output of the firewall during destruction */ 
 static int fw_quiet = 0;
diff --git a/src/gateway.c b/src/gateway.c
@@ -68,21 +68,11 @@
 static pthread_t tid_fw_counter = 0;
 static pthread_t tid_ping = 0;
 
-/* The internal web server */
-extern httpd *webserver;
-httpd *webserver = NULL;
-
-/* from commandline.c */
-extern char **restartargv;
-extern pid_t restart_orig_pid;
-
-/* from client_list.c */
-extern pthread_mutex_t client_list_mutex;
-
-/* Time when wifidog started  */
-extern time_t started_time;
 time_t started_time = 0;
 
+/* The internal web server */
+httpd * webserver = NULL;
+
 /* Appends -x, the current PID, and NULL to restartargv
  * see parse_commandline in commandline.c for details
  *
@@ -268,6 +258,7 @@ void
 termination_handler(int s)
 {
     static pthread_mutex_t sigterm_mutex = PTHREAD_MUTEX_INITIALIZER;
+    pthread_t self = pthread_self();
 
     debug(LOG_INFO, "Handler for termination caught signal %d", s);
 
@@ -287,11 +278,11 @@ termination_handler(int s)
      * termination handler) from happening so we need to explicitly kill the threads 
      * that use that
      */
-    if (tid_fw_counter) {
+    if (tid_fw_counter && self != tid_fw_counter) {
         debug(LOG_INFO, "Explicitly killing the fw_counter thread");
         pthread_kill(tid_fw_counter, SIGKILL);
     }
-    if (tid_ping) {
+    if (tid_ping && self != tid_ping) {
         debug(LOG_INFO, "Explicitly killing the ping thread");
         pthread_kill(tid_ping, SIGKILL);
     }
diff --git a/src/gateway.h b/src/gateway.h
@@ -28,6 +28,13 @@
 #ifndef _GATEWAY_H_
 #define _GATEWAY_H_
 
+#include "httpd.h"
+
+extern time_t started_time;
+
+/* The internal web server */
+extern httpd *webserver;
+
 /** @brief exits cleanly and clear the firewall rules. */
 void termination_handler(int s);
 
diff --git a/src/http.c b/src/http.c
@@ -57,7 +57,6 @@
 
 #include "../config.h"
 
-extern pthread_mutex_t client_list_mutex;
 
 /** The 404 handler is also responsible for redirecting to the auth server */
 void
diff --git a/src/ping_thread.c b/src/ping_thread.c
@@ -53,13 +53,11 @@
 #include "util.h"
 #include "centralserver.h"
 #include "firewall.h"
-
+#include "gateway.h"
 #include "simple_http.h"
 
 static void ping(void);
 
-extern time_t started_time;
-
 /** Launches a thread that periodically checks in with the wifidog auth server to perform heartbeat function.
 @param arg NULL
 @todo This thread loops infinitely, need a watchdog to verify that it is still running?
diff --git a/src/safe.c b/src/safe.c
@@ -19,9 +19,6 @@
  *                                                                  *
  \********************************************************************/
 
-/*
- * $Id$
- */
 /**
   @file safe.c
   @brief Safe versions of stdlib/string functions that error out and exit if memory allocation fails
@@ -36,14 +33,12 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <syslog.h>
 
 #include "httpd.h"
+#include "gateway.h"
 #include "safe.h"
 #include "debug.h"
-#include <syslog.h>
-
-/* From gateway.c */
-extern httpd *webserver;
 
 void *
 safe_malloc(size_t size)
diff --git a/src/simple_http.c b/src/simple_http.c
@@ -76,9 +76,11 @@ http_get(const int sockfd, const char *req)
 
     debug(LOG_DEBUG, "Sending HTTP request to auth server: [%s]\n", req);
     numbytes = send(sockfd, req, reqlen, 0);
-    // Cast numbytes to unsigned type because < 0 is already checked
-    if (numbytes < 0 || (size_t) numbytes != reqlen)  {
-        debug(LOG_ERR, "An error occurred while writing to server: %s", strerror(errno));
+    if (numbytes <= 0) {
+        debug(LOG_ERR, "send failed: %s", strerror(errno));
+        goto error;
+    } else if ((size_t) numbytes != reqlen) {
+        debug(LOG_ERR, "send failed: only %d bytes out of %d bytes sent!", numbytes, reqlen);
         goto error;
     }
 
@@ -104,6 +106,7 @@ http_get(const int sockfd, const char *req)
             } else if (numbytes == 0) {
                 done = 1;
             } else {
+                readbuf[numbytes] = '\0';
                 pstr_cat(response, readbuf);
                 debug(LOG_DEBUG, "Read %d bytes", numbytes);
             }
@@ -122,7 +125,9 @@ http_get(const int sockfd, const char *req)
     return retval;
 
  error:
-    close(sockfd);
+    if (sockfd >= 0) {
+        close(sockfd);
+    }
     retval = pstr_to_string(response);
     free(retval);
     return NULL;
@@ -291,6 +296,7 @@ https_get(const int sockfd, const char *req, const char *hostname)
                    connection. We can't distinguish between these cases right now. */
                 done = 1;
             } else {
+                readbuf[numbytes] = '\0';
                 pstr_cat(response, readbuf);
                 debug(LOG_DEBUG, "Read %d bytes", numbytes);
             }
@@ -315,7 +321,9 @@ https_get(const int sockfd, const char *req, const char *hostname)
     if (ssl) {
         CyaSSL_free(ssl);
     }
-    close(sockfd);
+    if (sockfd >= 0) {
+        close(sockfd);
+    }
     retval = pstr_to_string(response);
     free(retval);
     return NULL;
diff --git a/src/util.c b/src/util.c
diff --git a/src/util.h b/src/util.h
diff --git a/src/wdctl_thread.c b/src/wdctl_thread.c
