treewide: fix bugs from Coverity Scan

- Fix a couple of bugs in hardly ever (never?) used code paths that Coverity spotted
- Add comments for things that look a bit odd, but not sure if they are bugs

[cleanup commit message]
Signed-off-by: Nick Hainke <vincent@systemli.org>

src/include/datastorage.h

@@ -116,17 +116,18 @@ struct local_config_s {
     int loglevel;
 };
 
+// FIXME: Are these with or without NUL terminator?  Adjust values, string allocation and strncpy() to agree.
 #define MAX_IP_LENGTH 46
 #define MAX_KEY_LENGTH 65
 
 struct network_config_s {
-    char broadcast_ip[MAX_IP_LENGTH];
+    char broadcast_ip[MAX_IP_LENGTH + 1];
     int broadcast_port;
-    char server_ip[MAX_IP_LENGTH];
+    char server_ip[MAX_IP_LENGTH + 1];
     int tcp_port;
     int network_option;
-    char shared_key[MAX_KEY_LENGTH];
-    char iv[MAX_KEY_LENGTH];
+    char shared_key[MAX_KEY_LENGTH + 1];
+    char iv[MAX_KEY_LENGTH + 1];
     int use_symm_enc;
     int collision_domain;
     int bandwidth;
@@ -160,7 +161,7 @@ typedef struct probe_entry_s {
     uint8_t ht_capabilities; // eval_probe_metric()
     uint8_t vht_capabilities; // eval_probe_metric()
     time_t time; // remove_old...entries
-    int counter;
+    int counter; // FIXME: Never gets reset to zero.  Rely on deletion to create new for non-802.11k client?
     int deny_counter; // TODO: Never used?
     uint8_t max_supp_datarate; // TODO: Never used?
     uint8_t min_supp_datarate; // TODO: Never used?

src/include/utils.h

@@ -116,4 +116,4 @@ int _dawn_mutex_lock(pthread_mutex_t* m, char* f, int l);
 #define dawn_mutex_unlock(m) _dawn_mutex_unlock(m, __FILE__, __LINE__)
 int _dawn_mutex_unlock(pthread_mutex_t* m, char* f, int l);
 #endif
-#endif
+#endif

src/storage/datastorage.c

@@ -5,7 +5,6 @@
 #include "dawn_iwinfo.h"
 #include "dawn_uci.h"
 #include "mac_utils.h"
-#include "ieee80211_utils.h"
 
 #include "datastorage.h"
 #include "test_storage.h"
@@ -389,14 +388,12 @@ int better_ap_available(ap *kicking_ap, struct dawn_mac client_mac, struct kicki
         // then any more have to also be 'kicking_threshold' bigger
         if (score_to_compare > max_score + (kick ? 0 : dawn_metric.kicking_threshold)) {
             ap_outcome = 2; // Add and prune
-
             max_score = score_to_compare;
         }
         // if AP have same value but station count might improve it...
         // TODO: Is absolute number meaningful when AP have diffeent capacity?
         else if (score_to_compare == max_score && dawn_metric.use_station_count > 0 ) {
             int compare = compare_station_count(kicking_ap, candidate_ap, client_mac);
-
             if (compare > 0) {
                 ap_outcome = 2; // Add and prune
             }
@@ -542,14 +539,12 @@ int kick_clients(struct dawn_mac bssid_mac, uint32_t id) {
                 }
             }
         }
-        // no entry in probe array for own bssid
-        // TODO: Is test against -1 from (1 && -1) portable?
         else if (do_kick == -1) {
+            // FIXME: Causes clients to be kicked until first probe is received, which is a bit brutal for pre-802.11k clients.
             dawnlog_info("Station " MACSTR ": No Information about client. Force reconnect:\n", MAC2STR(j->client_addr.u8));
             print_client_entry(DAWNLOG_TRACE, j);
             del_client_interface(id, j->client_addr, 0, 1, 0);
         }
-        // ap is best
         else {
             dawnlog_info("Station " MACSTR ": Current AP is best. Client will stay:\n", MAC2STR(j->client_addr.u8));
             print_client_entry(DAWNLOG_TRACE, j);
@@ -577,10 +572,8 @@ void update_iw_info(struct dawn_mac bssid_mac) {
     dawn_mutex_lock(&client_array_mutex);
     dawn_mutex_lock(&probe_array_mutex);
 
-
     dawnlog_trace("IW info update for AP " MACSTR "\n", MAC2STR(bssid_mac.u8));
 
-
     // Go through clients for BSSID
     for (client* j = *client_find_first_bc_entry(bssid_mac, dawn_mac_null, false);
             j != NULL && mac_is_equal_bb(j->bssid_addr, bssid_mac); j = j->next_entry_bc) {

src/utils/mac_utils.c

@@ -70,7 +70,8 @@ void write_mac_to_file(char* path, struct dawn_mac addr) {
     if (f == NULL)
         dawnlog_error("Error opening mac file!\n");
     else
+    {   
         fprintf(f, MACSTR "\n", MAC2STR(addr.u8));
-
-    fclose(f);
+        fclose(f);
+    }
 }

src/utils/msghandler.c

@@ -225,6 +225,7 @@ probe_entry *parse_to_probe_req(struct blob_attr* msg) {
 int handle_deauth_req(struct blob_attr* msg) {
 
     dawnlog_debug_func("Entering...");
+
     hostapd_notify_entry notify_req;
     parse_to_hostapd_notify(msg, &notify_req);
 
@@ -330,7 +331,7 @@ int handle_network_msg(char* msg) {
     else if (strncmp(method, "macfile", 5) == 0) {
         parse_add_mac_to_file(data_buf.head);
     }
-    else if (strncmp(method, "uci", 2) == 0) {
+    else if (strncmp(method, "uci", 2) == 0) { // TODO: Should this be 3 or is something special happening?
         dawnlog_debug("HANDLING UCI!\n");
         handle_uci_config(data_buf.head);
     }

src/utils/ubus.c

@@ -87,7 +87,7 @@ struct hostapd_sock_entry {
     uint64_t last_channel_time_busy;
     int chan_util_samples_sum;
     int chan_util_num_sample_periods;
-    int chan_util_average; //TODO: Never evaluated?
+    int chan_util_average;
     int band;
 
     // add neighbor report string
@@ -684,11 +684,11 @@ static int hostapd_notify(struct ubus_context* ctx_local, struct ubus_object* ob
 }
 
 int dawn_init_ubus(const char *ubus_socket, const char *hostapd_dir) {
+    dawnlog_debug_func("Entering...");
+
     uloop_init();
     signal(SIGPIPE, SIG_IGN);
 
-    dawnlog_debug_func("Entering...");
-
     ctx = ubus_connect(ubus_socket);
     if (!ctx) {
         dawnlog_error("Failed to connect to ubus\n");
@@ -1170,7 +1170,7 @@ static void ubus_umdns_cb(struct ubus_request *req, int type, struct blob_attr *
             dawnlog_debug("IPV4: %s\n", blobmsg_get_string(tb_dawn[DAWN_UMDNS_IPV4]));
             dawnlog_debug("Port: %d\n", blobmsg_get_u32(tb_dawn[DAWN_UMDNS_PORT]));
         } else {
-            return;
+            return; // TODO: We're in a loop. Should this be return or continue?
         }
         add_tcp_connection(blobmsg_get_string(tb_dawn[DAWN_UMDNS_IPV4]), blobmsg_get_u32(tb_dawn[DAWN_UMDNS_PORT]));
     }
@@ -1460,7 +1460,6 @@ static void hostapd_handle_remove(struct ubus_context *ctx_local,
                                   struct ubus_subscriber *s, uint32_t id) {
     dawnlog_debug_func("HOSTAPD: Object %08x went away\n", id);
 
-
     struct hostapd_sock_entry *hostapd_sock = container_of(s,
     struct hostapd_sock_entry, subscriber);
 
@@ -1592,6 +1591,7 @@ void subscribe_to_new_interfaces(const char *hostapd_sock_path) {
         dawnlog_error("[SUBSCRIBING] No hostapd sockets!\n");
         return;
     }
+
     while ((entry = readdir(dirp)) != NULL) {
         if (entry->d_type == DT_SOCK) {
             bool do_subscribe = true;
@@ -2064,8 +2064,6 @@ void remove_probe_array_cb(struct uloop_timeout* t) {
     uloop_timeout_set(&probe_timeout, timeout_config.remove_probe * 1000);
 }
 
-
-
 void remove_client_array_cb(struct uloop_timeout* t) {
     dawnlog_debug_func("[ULOOP] : Removing old client entries!\n");
 
@@ -2076,8 +2074,6 @@ void remove_client_array_cb(struct uloop_timeout* t) {
     uloop_timeout_set(&client_timeout, timeout_config.remove_client * 1000);
 }
 
-
-
 void remove_ap_array_cb(struct uloop_timeout* t) {
     dawnlog_debug_func("[ULOOP] : Removing old ap entries!\n");