From 1bee8b3d288bc2c26432b8e675b18e482a98255e Mon Sep 17 00:00:00 2001 From: Dimitri Staessens Date: Mon, 28 May 2018 11:51:35 +0200 Subject: 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 Signed-off-by: Sander Vrijders --- CMakeLists.txt | 1 + src/ipcpd/normal/pol/graph.c | 45 ++++++++++++++++++++++++-------------------- src/ipcpd/udp/main.c | 36 +++++++++++++++++++++++++++++++---- 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; } -- cgit v1.2.3