build: Check for variable length arrays

The clang and gcc compilers don't complain about variable length
arrays using the -c89 flag unless the flag -Wvla or -Wpedantic is
set. This also fixes a memleak and two false positive uninitialized
variable warnings reported by the clang static analyzer in graph.c.

Signed-off-by: Dimitri Staessens <dimitri.staessens@ugent.be>
Signed-off-by: Sander Vrijders <sander.vrijders@ugent.be>

3 files changed, 58 insertions, 24 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d6d59229..535eef6c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -79,6 +79,7 @@ test_and_set_c_compiler_flag_global(-Werror)
 test_and_set_c_compiler_flag_global(-Wundef)
 test_and_set_c_compiler_flag_global(-Wpointer-arith)
 test_and_set_c_compiler_flag_global(-Wstrict-prototypes)
+test_and_set_c_compiler_flag_global(-Wvla)
 #Wswitch-default check fails on the swig-generated code
 #test_and_set_c_compiler_flag_global(-Wswitch-default)
 test_and_set_c_compiler_flag_global(-Wunreachable-code)
diff --git a/src/ipcpd/normal/pol/graph.c b/src/ipcpd/normal/pol/graph.c
index 007e1047..5aed1e9a 100644
--- a/src/ipcpd/normal/pol/graph.c
+++ b/src/ipcpd/normal/pol/graph.c
@@ -359,12 +359,7 @@ static int get_min_vertex(struct graph *   graph,
         *v = NULL;
 
         list_for_each(p, &graph->vertices) {
-                if (used[i]) {
-                        i++;
-                        continue;
-                }
-
-                if (dist[i] < min) {
+                if (!used[i] && dist[i] < min) {
                         min = dist[i];
                         index = i;
                         *v = list_entry(p, struct vertex, next);
@@ -384,7 +379,7 @@ static int dijkstra(struct graph *    graph,
                     struct vertex *** nhops,
                     int **            dist)
 {
-        bool               used[graph->nr_vertices];
+        bool *             used;
         struct list_head * p = NULL;
         int                i = 0;
         struct vertex *    v = NULL;
@@ -393,25 +388,24 @@ static int dijkstra(struct graph *    graph,
 
         *nhops = malloc(sizeof(**nhops) * graph->nr_vertices);
         if (*nhops == NULL)
-                return -1;
+                goto fail_pnhops;
 
         *dist = malloc(sizeof(**dist) * graph->nr_vertices);
-        if (*dist == NULL) {
-                free(*nhops);
-                return -1;
-        }
+        if (*dist == NULL)
+                goto fail_pdist;
+
+        used = malloc(sizeof(*used) * graph->nr_vertices);
+        if (used == NULL)
+                goto fail_used;
 
         /* Init the data structures */
+        memset(used, 0, sizeof(*used) * graph->nr_vertices);
+        memset(*nhops, 0, sizeof(**nhops) * graph->nr_vertices);
+        memset(*dist, 0, sizeof(**dist) * graph->nr_vertices);
+
         list_for_each(p, &graph->vertices) {
                 v = list_entry(p, struct vertex, next);
-                if (v->addr == src)
-                        (*dist)[i] = 0;
-                else
-                        (*dist)[i] = INT_MAX;
-
-                (*nhops)[i] = NULL;
-                used[i] = false;
-                i++;
+                (*dist)[i++]  = (v->addr == src) ? 0 : INT_MAX;
         }
 
         /* Perform actual Dijkstra */
@@ -441,7 +435,17 @@ static int dijkstra(struct graph *    graph,
                 i = get_min_vertex(graph, *dist, used, &v);
         }
 
+        free(used);
+
         return 0;
+
+ fail_used:
+        free(*dist);
+ fail_pdist:
+        free(*nhops);
+ fail_pnhops:
+        return -1;
+
 }
 
 static void free_routing_table(struct list_head * table)
@@ -536,6 +540,7 @@ static int graph_routing_table_simple(struct graph *     graph,
  fail_t:
         free_routing_table(table);
         free(nhops);
+        free(*dist);
  fail_vertices:
         *dist = NULL;
         return -1;
diff --git a/src/ipcpd/udp/main.c b/src/ipcpd/udp/main.c
index 26f993ce..8341aac1 100644
--- a/src/ipcpd/udp/main.c
+++ b/src/ipcpd/udp/main.c
@@ -788,7 +788,11 @@ static int ipcp_udp_reg(const uint8_t * hash)
         uint32_t dns_addr;
         uint32_t ip_addr;
 #endif
-        char hashstr[ipcp_dir_hash_strlen() + 1];
+        char * hashstr;
+
+        hashstr = malloc(ipcp_dir_hash_strlen() + 1);
+        if (hashstr == NULL)
+                return -1;
 
         assert(hash);
 
@@ -797,6 +801,7 @@ static int ipcp_udp_reg(const uint8_t * hash)
         if (shim_data_reg_add_entry(udp_data.shim_data, hash)) {
                 log_err("Failed to add " HASH_FMT " to local registry.",
                         HASH_VAL(hash));
+                free(hashstr);
                 return -1;
         }
 
@@ -810,11 +815,13 @@ static int ipcp_udp_reg(const uint8_t * hash)
 
                 if (inet_ntop(AF_INET, &ip_addr,
                               ipstr, INET_ADDRSTRLEN) == NULL) {
+                        free(hashstr);
                         return -1;
                 }
 
                 if (inet_ntop(AF_INET, &dns_addr,
                               dnsstr, INET_ADDRSTRLEN) == NULL) {
+                        free(hashstr);
                         return -1;
                 }
 
@@ -823,12 +830,15 @@ static int ipcp_udp_reg(const uint8_t * hash)
 
                 if (ddns_send(cmd)) {
                         shim_data_reg_del_entry(udp_data.shim_data, hash);
+                        free(hashstr);
                         return -1;
                 }
         }
 #endif
         log_dbg("Registered " HASH_FMT ".", HASH_VAL(hash));
 
+        free(hashstr);
+
         return 0;
 }
 
@@ -840,10 +850,14 @@ static int ipcp_udp_unreg(const uint8_t * hash)
         char cmd[100];
         uint32_t dns_addr;
 #endif
-        char hashstr[ipcp_dir_hash_strlen() + 1];
+        char * hashstr;
 
         assert(hash);
 
+        hashstr = malloc(ipcp_dir_hash_strlen() + 1);
+        if (hashstr == NULL)
+                return -1;
+
         ipcp_hash_str(hashstr, hash);
 
 #ifdef HAVE_DDNS
@@ -854,6 +868,7 @@ static int ipcp_udp_unreg(const uint8_t * hash)
         if (dns_addr != 0) {
                 if (inet_ntop(AF_INET, &dns_addr, dnsstr, INET_ADDRSTRLEN)
                     == NULL) {
+                        free(hashstr);
                         return -1;
                 }
                 sprintf(cmd, "server %s\nupdate delete %s A\nsend\nquit\n",
@@ -867,24 +882,32 @@ static int ipcp_udp_unreg(const uint8_t * hash)
 
         log_dbg("Unregistered " HASH_FMT ".", HASH_VAL(hash));
 
+        free(hashstr);
+
         return 0;
 }
 
 static int ipcp_udp_query(const uint8_t * hash)
 {
         uint32_t           ip_addr = 0;
+        char *             hashstr;
         struct hostent *   h;
 #ifdef HAVE_DDNS
         uint32_t           dns_addr = 0;
 #endif
-        char hashstr[ipcp_dir_hash_strlen() + 1];
 
         assert(hash);
 
+        hashstr = malloc(ipcp_dir_hash_strlen() + 1);
+        if (hashstr == NULL)
+                return -ENOMEM;
+
         ipcp_hash_str(hashstr, hash);
 
-        if (shim_data_dir_has(udp_data.shim_data, hash))
+        if (shim_data_dir_has(udp_data.shim_data, hash)) {
+                free(hashstr);
                 return 0;
+        }
 
 #ifdef HAVE_DDNS
         dns_addr = udp_data.dns_addr;
@@ -893,6 +916,7 @@ static int ipcp_udp_query(const uint8_t * hash)
                 ip_addr = ddns_resolve(hashstr, dns_addr);
                 if (ip_addr == 0) {
                         log_dbg("Could not resolve %s.", hashstr);
+                        free(hashstr);
                         return -1;
                 }
         } else {
@@ -900,6 +924,7 @@ static int ipcp_udp_query(const uint8_t * hash)
                 h = gethostbyname(hashstr);
                 if (h == NULL) {
                         log_dbg("Could not resolve %s.", hashstr);
+                        free(hashstr);
                         return -1;
                 }
 
@@ -910,9 +935,12 @@ static int ipcp_udp_query(const uint8_t * hash)
 
         if (shim_data_dir_add_entry(udp_data.shim_data, hash, ip_addr)) {
                 log_err("Failed to add directory entry.");
+                free(hashstr);
                 return -1;
         }
 
+        free(hashstr);
+
         return 0;
 }