Various fixes for the last 4 commits

Shorter NULL/zero conditions
Switch to libsodium random number generator
Fix of linkedlist_apply_if for no/false predicates
Fix use-after-free in linkedlist_move
Fix edge case memleak in create_p2p_hello
Typos, comments, a little bit of cleaning, etc.

src/coincerd.c

@@ -54,9 +54,6 @@ int main(void)
 		return 4;
 	}
 
-	/* TODO: use randombytes (from libsodium?) for the seed of randomness */
-	srand((unsigned) time(NULL));
-
 	if (global_state_init(&global_state)) {
 		log_error("Basic daemon setup");
 		return 1;
@@ -65,7 +62,7 @@ int main(void)
 	fetch_hosts(global_state.filepaths.hosts, &global_state.hosts);
 
 	/* setup everything needed for TCP listening */
-	if (listen_init(&listener, &global_state) != 0) {
+	if (listen_init(&listener, &global_state)) {
 		log_error("Initialization of TCP listening");
 		return 2;
 	}

src/configuration.c

@@ -37,7 +37,7 @@
 static int set_homedir(char **homedir)
 {
 	*homedir = getenv("HOME");
-	if (*homedir == NULL || *homedir[0] == '\0') {
+	if (!*homedir || *homedir[0] == '\0') {
 		log_error("Can not find home directory");
 		return 1;
 	}
@@ -61,7 +61,7 @@ static int set_directories(char **config_dir, char **data_dir)
 
 	/* configuration directory */
 	tmpchar = getenv("XDG_CONFIG_HOME");
-	if (tmpchar == NULL || tmpchar[0] == '\0') {
+	if (!tmpchar || tmpchar[0] == '\0') {
 		if (set_homedir(&homedir)) {
 			return 1;
 		}
@@ -69,7 +69,7 @@ static int set_directories(char **config_dir, char **data_dir)
 		*config_dir = (char *) malloc(tmpsize +
 					     sizeof("/.config/" PACKAGE_NAME
 						    "/"));
-		if (*config_dir == NULL) {
+		if (!*config_dir) {
 			log_error("Setting configdir");
 			return 1;
 		}
@@ -79,7 +79,7 @@ static int set_directories(char **config_dir, char **data_dir)
 		tmpsize = strlen(tmpchar);
 		*config_dir = (char *) malloc(tmpsize +
 					     sizeof("/" PACKAGE_NAME "/"));
-		if (*config_dir == NULL) {
+		if (!*config_dir) {
 			log_error("Setting configdir");
 			return 1;
 		}
@@ -89,8 +89,8 @@ static int set_directories(char **config_dir, char **data_dir)
 
 	/* data directory */
 	tmpchar = getenv("XDG_DATA_HOME");
-	if (tmpchar == NULL || tmpchar[0] == '\0') {
-		if (homedir == NULL && set_homedir(&homedir)) {
+	if (!tmpchar || tmpchar[0] == '\0') {
+		if (!homedir && set_homedir(&homedir)) {
 			free(config_dir);
 			return 1;
 		}
@@ -98,7 +98,7 @@ static int set_directories(char **config_dir, char **data_dir)
 		*data_dir = (char *) malloc(tmpsize +
 					   sizeof("/.local/share/" PACKAGE_NAME
 						  "/"));
-		if (*data_dir == NULL) {
+		if (!*data_dir) {
 			log_error("Setting datadir");
 			free(config_dir);
 			return 1;
@@ -109,7 +109,7 @@ static int set_directories(char **config_dir, char **data_dir)
 		tmpsize = strlen(tmpchar);
 		*data_dir = (char *) malloc(tmpsize +
 					   sizeof("/" PACKAGE_NAME "/"));
-		if (*data_dir == NULL) {
+		if (!*data_dir) {
 			log_error("Setting datadir");
 			free(config_dir);
 			return 1;

src/crypto.h

@@ -33,12 +33,10 @@ typedef struct s_keypair {
 void generate_keypair(keypair_t *keypair);
 
 uint32_t get_random_uint32_t(uint32_t upper_bound);
-
 uint64_t get_random_uint64_t(void);
 
 int hash_message(const char		*string_message,
 		 unsigned char		hash[crypto_generichash_BYTES]);
-
 void sign_message(const char		*string_message,
 		  const unsigned char	secret_key[crypto_box_SECRETKEYBYTES],
 		  unsigned char		signature[crypto_sign_BYTES]);

src/daemon_messages.c

@@ -55,15 +55,16 @@ int create_p2p_hello(message_t *message, unsigned short port)
 	p2p_hello_t	*hello;
 
 	hello = (p2p_hello_t *) malloc(sizeof(p2p_hello_t));
-	if (hello == NULL) {
+	if (!hello) {
 		log_error("Creating p2p.hello");
 		return 1;
 	}
 
 	client = (char *) malloc(sizeof("/" PACKAGE_NAME ":"
 					PACKAGE_VERSION "/"));
-	if (client == NULL) {
+	if (!client) {
 		log_error("Creating p2p.hello");
+		free(hello);
 		return 1;
 	}
 
@@ -92,12 +93,15 @@ int create_p2p_peers_adv(message_t *message, const linkedlist_t *hosts)
 	p2p_peers_adv_t *peers_adv;
 
 	peers_adv = (p2p_peers_adv_t *) malloc(sizeof(p2p_peers_adv_t));
-	if (peers_adv == NULL) {
+	if (!peers_adv) {
 		log_error("Creating p2p.peers.adv");
 		return 1;
 	}
 
-	hosts_to_str(hosts, &peers_adv->addresses);
+	if (hosts_to_str(hosts, &peers_adv->addresses)) {
+		log_error("Creating p2p.peers.adv");
+		return 1;
+	}
 
 	message->body.type = P2P_PEERS_ADV;
 	message->body.data = peers_adv;
@@ -179,7 +183,7 @@ int create_p2p_route_sol(message_t *message, const unsigned char *target)
 	p2p_route_sol_t *route_sol;
 
 	route_sol = (p2p_route_sol_t *) malloc(sizeof(p2p_route_sol_t));
-	if (route_sol == NULL) {
+	if (!route_sol) {
 		log_error("Creating p2p.route.sol");
 		return 1;
 	}

src/daemon_messages.h

@@ -86,20 +86,13 @@ typedef struct s_message {
 } message_t;
 
 int create_p2p_bye(message_t *message);
-
 int create_p2p_hello(message_t *message, unsigned short port);
-
 int create_p2p_peers_adv(message_t *message, const linkedlist_t *hosts);
-
 int create_p2p_peers_sol(message_t *message);
-
 int create_p2p_ping(message_t *message);
-
 int create_p2p_pong(message_t *message);
-
 int create_p2p_route_adv(message_t *message);
-
-int create_p2p_route_sol(message_t *msg_body, const unsigned char *target);
+int create_p2p_route_sol(message_t *message, const unsigned char *target);
 
 void message_delete(message_t *message);
 

src/hosts.c

@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "crypto.h"
 #include "hosts.h"
 #include "linkedlist.h"
 #include "log.h"
@@ -34,7 +35,7 @@
 void clear_hosts(linkedlist_t *hosts)
 {
 	/* host_t has no dynamically allocated variables */
-	linkedlist_destroy(hosts);
+	linkedlist_destroy(hosts, NULL);
 }
 
 /**
@@ -52,7 +53,7 @@ int fetch_hosts(const char *hosts_path, linkedlist_t *hosts)
 	FILE		*hosts_file;
 
 	hosts_file = fopen(hosts_path, "rb");
-	if (hosts_file == NULL) {
+	if (!hosts_file) {
 		log_warn("Hosts file not found at %s. "
 			 "It is safe to ignore this warning", hosts_path);
 		return 1;
@@ -88,21 +89,21 @@ int fetch_specific_hosts(const linkedlist_t	*hosts,
 	host_t			*current_host;
 	size_t			n = 0;
 
-	if (output != NULL) {
+	if (output) {
 		*output = (host_t **) malloc(linkedlist_size(hosts) *
 					     sizeof(host_t *));
-		if (*output == NULL) {
+		if (!*output) {
 			log_error("Fetching specific hosts");
 			return -1;
 		}
 	}
 
 	current_node = linkedlist_get_first(hosts);
-	while (current_node != NULL) {
+	while (current_node) {
 		current_host = (host_t *) current_node->data;
 		/* if all specified flags are being set on this host */
 		if ((current_host->flags & flags) == flags) {
-			if (output != NULL) {
+			if (output) {
 				(*output)[n++] = current_host;
 			} else {
 				n++;
@@ -128,7 +129,7 @@ host_t *find_host(const linkedlist_t	*hosts,
 {
 	const linkedlist_node_t *current = linkedlist_get_first(hosts);
 
-	while (current != NULL) {
+	while (current) {
 		host_t *current_data = (host_t *) current->data;
 
 		/* ip addresses match => requested host found */
@@ -142,21 +143,31 @@ host_t *find_host(const linkedlist_t	*hosts,
 	return NULL;
 }
 
-/* TODO: allocate memory for the 'output', don't assume any buffer size */
 /**
  * '\n' separated output string of host addresses in readable form.
  *
  * @param	hosts	List of hosts.
  * @param	output	Output string.
+ *
+ * @return	0	Success.
+ * @return	1	Failure.
  */
-void hosts_to_str(const linkedlist_t *hosts, char *output)
+int hosts_to_str(const linkedlist_t *hosts, char **output)
 {
 	linkedlist_node_t	*current_node;
 	host_t			*current_host;
 	size_t			output_size = 0;
 	char			text_ip[INET6_ADDRSTRLEN];
 
-	output[0] = '\0';
+	/* TODO: Don't assume any buffer size. This will be fixed in the
+	 * 	 next commit. */
+	*output = (char *) malloc(4096 * sizeof(char));
+	if (!*output) {
+		log_error("Hosts to string");
+		return 1;
+	}
+
+	*output[0] = '\0';
 	current_node = linkedlist_get_first(hosts);
 	while (current_node != NULL) {
 		current_host = (host_t *) current_node->data;
@@ -167,15 +178,17 @@ void hosts_to_str(const linkedlist_t *hosts, char *output)
 			  text_ip,
 			  INET6_ADDRSTRLEN);
 		output_size += strlen(text_ip);
-		strcat(output, text_ip);
+		strcat(*output, text_ip);
 
 		current_node = linkedlist_get_next(hosts, current_node);
 		/* if it's not the last host, append '\n' */
 		if (current_node != NULL) {
-			output[output_size++] = '\n';
+			*output[output_size++] = '\n';
 		}
-		output[output_size] = '\0';
+		*output[output_size] = '\0';
 	}
+
+	return 0;
 }
 
 /**
@@ -210,7 +223,7 @@ host_t *save_host(linkedlist_t *hosts, const struct in6_addr *addr)
 
 	/* allocate memory for a new host */
 	new_host = (host_t *) malloc(sizeof(host_t));
-	if (new_host == NULL) {
+	if (!new_host) {
 		log_error("Saving new host");
 		return NULL;
 	}
@@ -227,7 +240,7 @@ host_t *save_host(linkedlist_t *hosts, const struct in6_addr *addr)
 	 * function) is likely to save in ascending order => better performance
 	 */
 	current_node = linkedlist_get_last(hosts);
-	while (current_node != NULL) {
+	while (current_node) {
 		current_host = (host_t *) current_node->data;
 
 		cmp_value = memcmp(&new_host->addr, &current_host->addr, 16);
@@ -240,7 +253,7 @@ host_t *save_host(linkedlist_t *hosts, const struct in6_addr *addr)
 			new_node = linkedlist_insert_after(hosts,
 							   current_node,
 							   new_host);
-			if (new_node != NULL) {
+			if (new_node) {
 				log_debug("save_host - %s successfully saved",
 					  text_ip);
 			}
@@ -250,7 +263,7 @@ host_t *save_host(linkedlist_t *hosts, const struct in6_addr *addr)
 	}
 	/* the new host's addr is lexicographically the lowest */
 	new_node = linkedlist_insert_after(hosts, &hosts->first, new_host);
-	if (new_node != NULL) {
+	if (new_node) {
 		log_debug("save_host - %s successfully saved", text_ip);
 	}
 
@@ -284,7 +297,7 @@ void shuffle_hosts_arr(host_t **hosts, size_t hosts_size)
 		 * swapping hosts[i] with hosts[j], where j is a random index
 		 * such that j >= i AND j < hosts_size
 		 */
-		j = i + rand() % (hosts_size - i);
+		j = i + get_random_uint32_t(hosts_size - i);
 
 		tmp = hosts[i];
 		hosts[i] = hosts[j];
@@ -308,13 +321,13 @@ int store_hosts(const char *hosts_path, const linkedlist_t *hosts)
 	FILE			*hosts_file;
 
 	hosts_file = fopen(hosts_path, "wb");
-	if (hosts_file == NULL) {
+	if (!hosts_file) {
 		log_error("Can not create hosts file at %s", hosts_path);
 		return 1;
 	}
 
 	current = linkedlist_get_first(hosts);
-	while (current != NULL) {
+	while (current) {
 		current_host = (host_t *) current->data;
 		/* if fwrite fails, terminate storing */
 		if (fwrite(&current_host->addr,

src/hosts.h

@@ -24,18 +24,16 @@
 
 #include "linkedlist.h"
 
-/* number of hosts guaranteed to be in the network */
+/** The number of hosts guaranteed to be in the network. */
 #define	DEFAULT_HOSTS_SIZE	2
-/* maximum number of hosts we store */
+/** Maximum number of hosts we store. */
 #define	MAX_HOSTS_SIZE		50
 
 /** A host is not available if they is already our neighbour,
- *  pending to become one, or if we are unnable to connect to them.
- *  1 if available, 0 if not.
- */
+ *  pending to become one, or if we are unnable to connect to them. */
 #define HOST_AVAILABLE	0x01
 
-/* IPv6 addresses of hosts guaranteed to be in the network */
+/** IPv6 addresses of hosts guaranteed to be in the network. */
 static const unsigned char DEFAULT_HOSTS[DEFAULT_HOSTS_SIZE][16] = {
 /* TODO: Replace with real default hosts */
 	{
@@ -72,7 +70,7 @@ int fetch_specific_hosts(const linkedlist_t	*hosts,
 host_t *find_host(const linkedlist_t	*hosts,
 		  const struct in6_addr	*addr);
 
-void hosts_to_str(const linkedlist_t *hosts, char *output);
+int hosts_to_str(const linkedlist_t *hosts, char **output);
 
 void reset_hosts_availability(linkedlist_t *hosts);
 

src/json_parser.h

@@ -21,6 +21,8 @@
 
 #include "daemon_messages.h"
 
+/* the order of these strings must be the same as the order of enums within
+ * enum message_type at daemon_messages.h */
 static const char *msg_type_str[] = {
 	"p2p.bye",
 	"p2p.hello",

src/linkedlist.c

@@ -82,15 +82,15 @@ void linkedlist_apply_if(linkedlist_t	*root,
 	void			*node_data;
 
 	current_node = linkedlist_get_first(root);
-	while (current_node != NULL) {
+	while (current_node) {
 		node_data = current_node->data;
+		next_node = linkedlist_get_next(root, current_node);
 
-		if (pred == NULL || pred(node_data, attribute)) {
-			if (data_func != NULL) {
+		if (!pred || pred(node_data, attribute)) {
+			if (data_func) {
 				data_func(current_node->data);
 			}
-			next_node = linkedlist_get_next(root, current_node);
-			if (node_func != NULL) {
+			if (node_func) {
 				node_func(current_node);
 			}
 		}
@@ -126,8 +126,8 @@ void linkedlist_delete_safely(linkedlist_node_t *node,
 	node->next->prev = node->prev;
 
 	/* node's data deletion part */
-	if (node->data != NULL) {
-		if (clear_data != NULL) {
+	if (node->data) {
+		if (clear_data) {
 			clear_data(node->data);
 		}
 		free(node->data);
@@ -151,10 +151,10 @@ void linkedlist_destroy(linkedlist_t	*root,
 	linkedlist_node_t *tmp;
 
 	tmp = root->first.next;
-	while (tmp->next != NULL) {
+	while (tmp->next) {
 		tmp = tmp->next;
-		if (tmp->prev->data != NULL) {
-			if (clear_data != NULL) {
+		if (tmp->prev->data) {
+			if (clear_data) {
 				clear_data(tmp->prev->data);
 			}
 			free(tmp->prev->data);
@@ -188,7 +188,7 @@ linkedlist_node_t *linkedlist_find(const linkedlist_t	*root,
 {
 	/* start the search from the first node of the linked list */
 	linkedlist_node_t *current = linkedlist_get_first(root);
-	while (current != NULL) {
+	while (current) {
 		/* node found */
 		if (current->data == data) {
 			/* return the node containing 'data' */
@@ -248,7 +248,7 @@ linkedlist_node_t *linkedlist_get_last(const linkedlist_t *root)
 linkedlist_node_t *linkedlist_get_next(const linkedlist_t	*root,
 				       const linkedlist_node_t	*node)
 {
-	if (node == NULL || node->next == &root->last) {
+	if (!node || node->next == &root->last) {
 		return NULL;
 	}
 
@@ -268,7 +268,7 @@ linkedlist_node_t *linkedlist_get_next(const linkedlist_t	*root,
 linkedlist_node_t *linkedlist_get_prev(const linkedlist_t	*root,
 				       const linkedlist_node_t	*node)
 {
-	if (node == NULL || node->prev == &root->first) {
+	if (!node || node->prev == &root->first) {
 		return NULL;
 	}
 
@@ -313,7 +313,7 @@ linkedlist_node_t *linkedlist_insert_after(linkedlist_t		*root,
 	}
 
 	new_node = (linkedlist_node_t *) malloc(sizeof(linkedlist_node_t));
-	if (new_node == NULL) {
+	if (!new_node) {
 		log_error("Inserting a new node");
 		return NULL;
 	}
@@ -359,8 +359,9 @@ void linkedlist_move(linkedlist_node_t *node, linkedlist_t *dest)
 {
 	linkedlist_node_t *dest_last;
 
-	/* remove the node from its LL */
-	linkedlist_remove(node);
+	/* unlink the node from its LL */
+	node->prev->next = node->next;
+	node->next->prev = node->prev;
 
 	/* insert the node right before the last (stub) node of the dest LL */
 	dest_last = dest->last.prev;
@@ -395,7 +396,7 @@ void linkedlist_remove_all(linkedlist_t *root)
 	linkedlist_node_t *tmp;
 
 	tmp = root->first.next;
-	while (tmp->next != NULL) {
+	while (tmp->next) {
 		tmp = tmp->next;
 		free(tmp->prev);
 	}
@@ -416,7 +417,7 @@ size_t linkedlist_size(const linkedlist_t *root)
 	size_t			n = 0;
 
 	tmp = root->first.next;
-	while (tmp->next != NULL) {
+	while (tmp->next) {
 		n++;
 		tmp = tmp->next;
 	}

src/neighbours.c

@@ -47,7 +47,7 @@ neighbour_t *add_new_neighbour(linkedlist_t		*neighbours,
 	/* create new neighbour */
 	new_neighbour = (neighbour_t *) malloc(sizeof(neighbour_t));
 	/* allocation failure */
-	if (new_neighbour == NULL) {
+	if (!new_neighbour) {
 		log_error("add_new_neighbour - new_neighbour malloc");
                 return NULL;
 	}
@@ -117,8 +117,6 @@ int compare_neighbour_bufferevents(const neighbour_t		*neighbour,
 	return neighbour->buffer_event != bev;
 }
 
-}
-
 /**
  * Fetch pointers to neighbours with specific flags set, into an array
  * that is being allocated in here.
@@ -141,21 +139,21 @@ int fetch_specific_neighbours(const linkedlist_t	*neighbours,
 	neighbour_t		*current_neighbour;
 	size_t			n = 0;
 
-	if (output != NULL) {
+	if (output) {
 		*output = (neighbour_t **) malloc(linkedlist_size(neighbours) *
 						  sizeof(neighbour_t *));
-		if (*output == NULL) {
+		if (!*output) {
 			log_error("Fetching specific neighbours");
 			return -1;
 		}
 	}
 
 	current_node = linkedlist_get_first(neighbours);
-	while (current_node != NULL) {
+	while (current_node) {
 		current_neighbour = (neighbour_t *) current_node->data;
 		/* if all specified flags are being set on this neighbour */
 		if ((current_neighbour->flags & flags) == flags) {
-			if (output != NULL) {
+			if (output) {
 				(*output)[n++] = current_neighbour;
 			} else {
 				n++;
@@ -187,7 +185,7 @@ neighbour_t *find_neighbour(const linkedlist_t	*neighbours,
 	neighbour_t		*current_neighbour;
 
 	current_node = linkedlist_get_first(neighbours);
-	while (current_node != NULL) {
+	while (current_node) {
 		current_neighbour = (neighbour_t *) current_node->data;
 
 		/* cmp_func returns 0 when current_neighbour's attribute

src/neighbours.h

@@ -26,9 +26,6 @@
 #include "hosts.h"
 #include "linkedlist.h"
 
-/** Minimum number of peers we need to be connected to. */
-#define MIN_NEIGHBOURS	3
-
 /** Request for addresses. */
 #define NEIGHBOUR_ADDRS_REQ	0x01
 

src/p2p.c

@@ -29,6 +29,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "crypto.h"
+#include "global_state.h"
 #include "hosts.h"
 #include "linkedlist.h"
 #include "log.h"
@@ -56,7 +58,7 @@ static void ask_for_addresses(neighbour_t *neighbour)
 {
 	struct bufferevent *bev;
 
-	if (neighbour == NULL || (bev = neighbour->buffer_event) == NULL) {
+	if (!neighbour || !(bev = neighbour->buffer_event)) {
 		return;
 	}
 
@@ -82,7 +84,7 @@ static void process_hosts(global_state_t *global_state, char *hosts)
 
 	line = strtok_r(hosts, delim, &save_ptr);
 
-	while (line != NULL) {
+	while (line) {
 		if (inet_pton(AF_INET6, line, &addr) == 1) {
 			save_host(&global_state->hosts, &addr);
 		}
@@ -98,6 +100,7 @@ static void process_hosts(global_state_t *global_state, char *hosts)
  */
 static void p2p_process(struct bufferevent *bev, void *ctx)
 {
+	char		*addrs = NULL;
 	global_state_t	*global_state;
 	struct evbuffer	*input;
 	size_t		len;
@@ -127,7 +130,7 @@ static void p2p_process(struct bufferevent *bev, void *ctx)
 
 	/* allocate memory for the input message including '\0' */
 	message = (char *) malloc((len + 1) * sizeof(char));
-	if (message == NULL) {
+	if (!message) {
 		log_error("Received message allocation");
 		return;
 	}
@@ -152,7 +155,7 @@ static void p2p_process(struct bufferevent *bev, void *ctx)
 	} else if (strcmp(message, "pong") == 0) {
 	/* "hosts" is a request for list of addresses */
 	} else if (strcmp(message, "hosts") == 0) {
-		hosts_to_str(&global_state->hosts, response);
+		hosts_to_str(&global_state->hosts, &addrs);
 	/* list of addresses */
 	} else {
 		if (neighbour->flags & NEIGHBOUR_ADDRS_REQ) {
@@ -165,6 +168,13 @@ static void p2p_process(struct bufferevent *bev, void *ctx)
 		log_debug("p2p_process - responding with: %s", response);
 		/* copy response to the output buffer */
 		evbuffer_add_printf(output, "%s", response);
+	/* note: this will be replaced in the next commit */
+	} else if (addrs) {
+		log_debug("p2p_process - responding with: %s", addrs);
+		/* copy response to the output buffer */
+		evbuffer_add_printf(output, "%s", addrs);
+
+		free(addrs);
 	}
 
 	/* deallocate input message */
@@ -242,7 +252,7 @@ static void process_pending_neighbour(global_state_t	*global_state,
 		if (needed_conns > 0) {
 			/* and we don't have enough hosts available */
 			if (available_hosts_size < needed_conns) {
-				ask_for_addresses(new_neighbour);
+				ask_for_addresses(neighbour);
 			}
 		}
 	/* connecting to the neighbour was unsuccessful */
@@ -298,7 +308,7 @@ static void event_cb(struct bufferevent *bev, short events, void *ctx)
 	neighbour = find_neighbour(&global_state->neighbours,
 				   bev,
 				   compare_neighbour_bufferevents);
-	if (neighbour != NULL) {
+	if (neighbour) {
 		process_neighbour(global_state, neighbour, events);
 	/* no such neighbour found; try finding it at pending_neighbours */
 	} else {
@@ -383,10 +393,10 @@ static void accept_connection(struct evconnlistener *listener,
 	host = save_host(&global_state->hosts, new_addr);
 	if (!host) {