From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Tue, 23 Apr 2019 21:57:04 +0000 (-0500)
Subject: Random TODO patching
X-Git-Tag: v0.0.2~46^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=76f0f9dde4beebb56cdcc4c848beca0b145d8645;p=thirdparty%2FFORT-validator.git

Random TODO patching
---

diff --git a/src/certificate_refs.c b/src/certificate_refs.c
index 13db4d57..f3ddf2a3 100644
--- a/src/certificate_refs.c
+++ b/src/certificate_refs.c
@@ -77,14 +77,20 @@ validate_signedObject(struct certificate_refs *refs,
 	return 0;
 }
 
+/**
+ * Ensures the @refs URIs match the parent Manifest's URIs. Assumes @refs came
+ * from a CA certificate.
+ *
+ * @refs: References you want validated.
+ * @pp: Repository Publication Point, as described by the parent Manifest.
+ */
 int
-refs_validate_ca(struct certificate_refs *refs, bool is_ta,
-    struct rpp const *pp)
+refs_validate_ca(struct certificate_refs *refs, struct rpp const *pp)
 {
 	int error;
 
-	if (is_ta)
-		return 0;
+	if (pp == NULL)
+		return 0; /* This CA is the TA, and therefore lacks a parent. */
 
 	error = validate_cdp(refs, pp);
 	if (error)
@@ -102,6 +108,14 @@ refs_validate_ca(struct certificate_refs *refs, bool is_ta,
 	return 0;
 }
 
+/**
+ * Ensures the @refs URIs match the Manifest URIs. Assumes @refs came from an
+ * EE certificate.
+ *
+ * @refs: References you want validated.
+ * @pp: Repository Publication Point, as described by the Manifest.
+ * @uri: URL of the signed object that contains the EE certificate.
+ */
 int
 refs_validate_ee(struct certificate_refs *refs, struct rpp const *pp,
     struct rpki_uri const *uri)
diff --git a/src/certificate_refs.h b/src/certificate_refs.h
index 55d55c3c..a2dc8818 100644
--- a/src/certificate_refs.h
+++ b/src/certificate_refs.h
@@ -1,7 +1,6 @@
 #ifndef SRC_CERTIFICATE_REFS_H_
 #define SRC_CERTIFICATE_REFS_H_
 
-#include <stdbool.h>
 #include "rpp.h"
 
 /**
@@ -39,7 +38,7 @@ struct certificate_refs {
 
 void refs_init(struct certificate_refs *);
 void refs_cleanup(struct certificate_refs *);
-int refs_validate_ca(struct certificate_refs *, bool, struct rpp const *);
+int refs_validate_ca(struct certificate_refs *, struct rpp const *);
 int refs_validate_ee(struct certificate_refs *, struct rpp const *,
     struct rpki_uri const *);
 
diff --git a/src/clients.c b/src/clients.c
index bd072f37..54431964 100644
--- a/src/clients.c
+++ b/src/clients.c
@@ -11,25 +11,18 @@ ARRAY_LIST(clientsdb, struct client)
 static struct clientsdb clients_db;
 
 /* Read and Write locks */
-sem_t rlock, wlock;
+static sem_t rlock, wlock;
 
 /* Readers counter */
-unsigned int rcounter;
+static unsigned int rcounter;
 
-int
+void
 clients_db_init(void)
 {
-	int error;
-
-	error = clientsdb_init(&clients_db);
-	if (error)
-		return error;
-
+	clientsdb_init(&clients_db);
 	sem_init(&rlock, 0, 1);
 	sem_init(&wlock, 0, 1);
 	rcounter = 0;
-
-	return error;
 }
 
 static struct client *
@@ -45,6 +38,7 @@ get_client(struct sockaddr_storage *addr)
 				    SADDR_IN(addr)->sin_addr.s_addr &&
 				    ptr->sin_port ==
 				    SADDR_IN(addr)->sin_port) {
+					/* TODO (urgent) incorrect locking */
 					read_unlock(&rlock, &wlock, &rcounter);
 					return ptr;
 				}
@@ -125,45 +119,35 @@ client_list(struct client **clients)
 	len = clients_db.len;
 	read_unlock(&rlock, &wlock, &rcounter);
 
+	/* TODO (urgent) incorrect locking */
 	return len;
 }
 
-static void
-client_destroy(struct client *client)
-{
-	/* Didn't allocate something, so do nothing */
-}
-
 void
 clients_forget(int fd)
 {
-	struct clientsdb *new_db;
+	struct clientsdb new_db;
 	struct client *ptr;
 
-	new_db = malloc(sizeof(struct clientsdb));
-	if (new_db == NULL) {
-		pr_err("Couldn't allocate new clients DB");
-		return; /* TODO This is not acceptable... */
-	}
-	clientsdb_init(new_db);
+	clientsdb_init(&new_db);
+	/* TODO (urgent) incorrect locking */
 	read_lock(&rlock, &wlock, &rcounter);
 	ARRAYLIST_FOREACH(&clients_db, ptr)
 		if (ptr->fd != fd)
-			clientsdb_add(new_db, ptr);
+			clientsdb_add(&new_db, ptr);
 	read_unlock(&rlock, &wlock, &rcounter);
 
 	sem_wait(&wlock);
-	clientsdb_cleanup(&clients_db, client_destroy);
-	clients_db = *new_db;
+	clientsdb_cleanup(&clients_db, NULL);
+	clients_db = new_db;
 	sem_post(&wlock);
-	free(new_db);
 }
 
 void
 clients_db_destroy(void)
 {
 	sem_wait(&wlock);
-	clientsdb_cleanup(&clients_db, client_destroy);
+	clientsdb_cleanup(&clients_db, NULL);
 	sem_post(&wlock);
 
 	sem_destroy(&wlock);
diff --git a/src/clients.h b/src/clients.h
index db1a460b..41b88ccc 100644
--- a/src/clients.h
+++ b/src/clients.h
@@ -14,7 +14,7 @@ struct client {
 	uint8_t rtr_version;
 };
 
-int clients_db_init(void);
+void clients_db_init(void);
 int update_client(int, struct sockaddr_storage *, uint8_t);
 size_t client_list(struct client **);
 
diff --git a/src/config.c b/src/config.c
index bae3f2b2..f4c7aaa3 100644
--- a/src/config.c
+++ b/src/config.c
@@ -26,7 +26,7 @@
  * Assuming you don't need to create a data type, that should be all.
  */
 struct rpki_config {
-	/** TAL file name or directory. TODO support directories */
+	/** TAL file name or directory. TODO (urgent) support directories */
 	char *tal;
 	/** Path of our local clone of the repository */
 	char *local_repository;
@@ -45,16 +45,15 @@ struct rpki_config {
 	unsigned int maximum_certificate_depth;
 
 	struct {
-		/** The listener address of the RTR server. */
+		/** The bound listening address of the RTR server. */
 		char *address;
-		/** TODO document */
+		/** The bound listening port of the RTR server. */
 		char *port;
 		/** Maximum accepted client connections */
 		unsigned int queue;
 
 		/** Interval used to look for updates at VRPs location */
-		/* TODO rename */
-		unsigned int vrps_check_interval;
+		unsigned int validation_interval;
 
 		/** Intervals use at RTR v1 End of data PDU **/
 		uint32_t refresh_interval;
@@ -183,13 +182,13 @@ static const struct option_field options[] = {
 		.name = "server.address",
 		.type = &gt_string,
 		.offset = offsetof(struct rpki_config, server.address),
-		.doc = "The listener address of the RTR server.",
+		.doc = "Address the RTR server will bind itself to. Can be a name, in which case an address will be resolved.",
 	}, {
 		.id = 5001,
 		.name = "server.port",
 		.type = &gt_string,
 		.offset = offsetof(struct rpki_config, server.port),
-		.doc = "", /* TODO */
+		.doc = "Port the RTR server will bind itself to. Can be a string, in which case a number will be resolved.",
 	}, {
 		.id = 5003,
 		.name = "server.queue",
@@ -200,9 +199,10 @@ static const struct option_field options[] = {
 		.max = SOMAXCONN,
 	}, {
 		.id = 5004,
-		.name = "server.vrps-check-interval",
+		.name = "server.validation-interval",
 		.type = &gt_uint,
-		.offset = offsetof(struct rpki_config, server.vrps_check_interval),
+		.offset = offsetof(struct rpki_config,
+		    server.validation_interval),
 		.doc = "Interval used to look for updates at VRPs location",
 		/*
 		 * RFC 6810 and 8210:
@@ -431,7 +431,7 @@ set_default_values(void)
 		return pr_enomem();
 
 	rpki_config.server.queue = 10;
-	rpki_config.server.vrps_check_interval = 60;
+	rpki_config.server.validation_interval = 60;
 	rpki_config.server.refresh_interval = 3600;
 	rpki_config.server.retry_interval = 600;
 	rpki_config.server.expire_interval = 7200;
@@ -621,9 +621,9 @@ config_get_server_queue(void)
 }
 
 unsigned int
-config_get_vrps_check_interval(void)
+config_get_validation_interval(void)
 {
-	return rpki_config.server.vrps_check_interval;
+	return rpki_config.server.validation_interval;
 }
 
 uint32_t
@@ -708,8 +708,7 @@ config_get_rsync_args(bool is_ta)
 		break;
 	}
 
-	pr_crit("Invalid sync strategy: '%u'",
-	    rpki_config.sync_strategy);
+	pr_crit("Invalid sync strategy: '%u'", rpki_config.sync_strategy);
 	/*
 	 * Return something usable anyway; don't want to check NULL.
 	 * This is supposed to be unreachable code anyway.
diff --git a/src/config.h b/src/config.h
index 1eca1e09..c2e470f3 100644
--- a/src/config.h
+++ b/src/config.h
@@ -17,7 +17,7 @@ void free_rpki_config(void);
 char const *config_get_server_address(void);
 char const *config_get_server_port(void);
 int config_get_server_queue(void);
-unsigned int config_get_vrps_check_interval(void);
+unsigned int config_get_validation_interval(void);
 uint32_t config_get_refresh_interval(void);
 uint32_t config_get_retry_interval(void);
 uint32_t config_get_expire_interval(void);
diff --git a/src/config/string_array.c b/src/config/string_array.c
index ccbb6803..c0ccae9d 100644
--- a/src/config/string_array.c
+++ b/src/config/string_array.c
@@ -16,6 +16,11 @@ string_array_init(struct string_array *array, char const *const *values,
 
 	array->length = len;
 
+	if (len == 0) {
+		array->array = NULL;
+		return 0;
+	}
+
 	array->array = calloc(len, sizeof(char *));
 	if (array->array == NULL)
 		return -ENOMEM;
diff --git a/src/config/string_array.h b/src/config/string_array.h
index ce0a836f..14e0eb9d 100644
--- a/src/config/string_array.h
+++ b/src/config/string_array.h
@@ -5,10 +5,7 @@
 #include "config/types.h"
 
 struct string_array {
-	/*
-	 * BTW: The array size can be zero, in which case this will be NULL.
-	 * TODO Remember to handle properly.
-	 */
+	/* BTW: The array size can be zero, in which case this will be NULL. */
 	char **array;
 	size_t length;
 };
diff --git a/src/data_structure/array_list.h b/src/data_structure/array_list.h
index 595600c5..a4f5e089 100644
--- a/src/data_structure/array_list.h
+++ b/src/data_structure/array_list.h
@@ -6,10 +6,9 @@
 #include "log.h"
 #include "data_structure/common.h"
 
-/* TODO sizes used to be unsigned ints. Check callers. */
 #define DEFINE_ARRAY_LIST_STRUCT(name, elem_type)			\
 	struct name {							\
-		/** Unidimensional array. */				\
+		/** Unidimensional array. Initialized lazily. */	\
 		elem_type *array;					\
 		/** Number of elements in @array. */			\
 		size_t len;						\
@@ -18,22 +17,18 @@
 	}
 
 #define DEFINE_ARRAY_LIST_FUNCTIONS(name, elem_type)			\
-	static int							\
+	static void							\
 	name##_init(struct name *list)					\
 	{								\
-		list->capacity = 8;					\
+		list->array = NULL;					\
 		list->len = 0;						\
-		/* TODO I need lazy initialization of this badly */	\
-		list->array = malloc(list->capacity			\
-		    * sizeof(elem_type));				\
-		return (list->array != NULL) ? 0 : pr_enomem();		\
+		list->capacity = 0;					\
 	}								\
 									\
 	static void							\
 	name##_cleanup(struct name *list, void (*cb)(elem_type *))	\
 	{								\
 		array_index i;						\
-		/* TODO recently added this. Use it more */		\
 		if (cb != NULL)						\
 			for (i = 0; i < list->len; i++)			\
 				cb(&list->array[i]);			\
@@ -46,6 +41,14 @@
 	{								\
 		elem_type *tmp;						\
 									\
+		if (list->array == NULL) {				\
+			list->capacity = 8;				\
+			list->array = malloc(list->capacity		\
+			    * sizeof(elem_type));			\
+			if (list->array == NULL)			\
+				return pr_enomem();			\
+		}							\
+									\
 		list->len++;						\
 		while (list->len >= list->capacity) {			\
 			list->capacity *= 2;				\
@@ -65,10 +68,11 @@
 	DEFINE_ARRAY_LIST_STRUCT(name, elem_type);			\
 	DEFINE_ARRAY_LIST_FUNCTIONS(name, elem_type)
 
-#define ARRAYLIST_FOREACH(list, cursor) for (				\
-	cursor = (list)->array;						\
-	(cursor - ((typeof(cursor)) ((list)->array))) < (list)->len;	\
-	cursor++							\
+/* c = cursor */
+#define ARRAYLIST_FOREACH(list, c) for (				\
+	(c) = (list)->array;						\
+	((c) != NULL) && (((c) - (typeof(c)) ((list)->array)) < (list)->len); \
+	(c)++								\
 )
 
 #endif /* SRC_DATA_STRUCTURE_ARRAY_LIST_H_ */
diff --git a/src/data_structure/circular_indexer.c b/src/data_structure/circular_indexer.c
index f12a3ca6..d405137a 100644
--- a/src/data_structure/circular_indexer.c
+++ b/src/data_structure/circular_indexer.c
@@ -142,23 +142,3 @@ success:
 	indexer->len--;
 	return 0;
 }
-
-void
-arridx_print(char const *prefix, struct circular_indexer *i)
-{
-	array_index o;
-	array_index p;
-
-	pr_info("%s:", prefix);
-	if (i->indexes != NULL) {
-		pr_info("  indexes:");
-		p = i->first;
-		for (o = 0; o < i->len; o++) {
-			pr_info("    %zu", p);
-			p = i->indexes[p].next;
-		}
-	}
-
-	pr_info("  first:%zu current:%zu top:%zu len:%zu allow:%d", i->first,
-	    i->current, i->top, i->len, i->allow_another_lap);
-}
diff --git a/src/data_structure/circular_indexer.h b/src/data_structure/circular_indexer.h
index f065d8e3..b79db979 100644
--- a/src/data_structure/circular_indexer.h
+++ b/src/data_structure/circular_indexer.h
@@ -102,7 +102,4 @@ array_index *arridx_next(struct circular_indexer *);
 /* Removes the *current* element. (You must be iterating.) */
 int arridx_remove(struct circular_indexer *);
 
-/* TODO remove me */
-void arridx_print(char const *, struct circular_indexer *);
-
 #endif /* SRC_DATA_STRUCTURE_CIRCULAR_INDEXER_H_ */
diff --git a/src/main.c b/src/main.c
index 909eead7..73cd3a5f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -14,20 +14,15 @@ start_rtr_server(void)
 {
 	int error;
 
-	error = vrps_init();
-	if (error)
-		goto end1;
-
-	error = clients_db_init();
-	if (error)
-		goto end2;
+	vrps_init();
+	clients_db_init();
 
 	error = rtr_listen();
 	rtr_cleanup(); /* TODO shouldn't this only happen on !error? */
 
 	clients_db_destroy();
-end2:	vrps_destroy();
-end1:	return error;
+	vrps_destroy();
+	return error;
 }
 
 int
diff --git a/src/object/certificate.c b/src/object/certificate.c
index fe02b769..0d728eff 100644
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -643,6 +643,9 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls)
 	int ok;
 	int error;
 
+	if (crls == NULL)
+		return 0; /* Certificate is TA; no chain validation needed. */
+
 	state = state_retrieve();
 	if (state == NULL)
 		return -EINVAL;
@@ -661,8 +664,7 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls)
 	}
 
 	X509_STORE_CTX_trusted_stack(ctx, validation_certs(state));
-	if (crls != NULL)
-		X509_STORE_CTX_set0_crls(ctx, crls);
+	X509_STORE_CTX_set0_crls(ctx, crls);
 
 	/*
 	 * HERE'S THE MEAT OF LIBCRYPTO'S VALIDATION.
@@ -1437,8 +1439,11 @@ certificate_validate_extensions_ee(X509 *cert, OCTET_STRING_t *sid,
 /* Boilerplate code for CA certificate validation and recursive traversal. */
 int
 certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri,
-    STACK_OF(X509_CRL) *crls, bool is_ta)
+    STACK_OF(X509_CRL) *crls)
 {
+/** Is the CA certificate the TA certificate? */
+#define IS_TA (rpp_parent == NULL)
+
 	struct validation *state;
 	X509 *cert;
 	struct rfc5280_name *subject_name;
@@ -1463,26 +1468,24 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri,
 	error = certificate_load(cert_uri, &cert);
 	if (error)
 		goto revert_fnstack_and_debug;
-	if (!is_ta) {
-		error = certificate_validate_chain(cert, crls);
-		if (error)
-			goto revert_cert;
-	}
-	error = certificate_validate_rfc6487(cert, &subject_name, is_ta);
+	error = certificate_validate_chain(cert, crls);
+	if (error)
+		goto revert_cert;
+	error = certificate_validate_rfc6487(cert, &subject_name, IS_TA);
 	if (error)
 		goto revert_cert;
-	error = is_ta
+	error = IS_TA
 	    ? certificate_validate_extensions_ta(cert, &mft, &policy)
 	    : certificate_validate_extensions_ca(cert, &mft, &refs, &policy);
 	if (error)
 		goto revert_subject_name;
 
-	error = refs_validate_ca(&refs, is_ta, rpp_parent);
+	error = refs_validate_ca(&refs, rpp_parent);
 	if (error)
 		goto revert_uri_and_refs;
 
 	/* -- Validate the manifest (@mft) pointed by the certificate -- */
-	error = validation_push_cert(state, cert_uri, cert, policy, is_ta);
+	error = validation_push_cert(state, cert_uri, cert, policy, IS_TA);
 	if (error)
 		goto revert_uri_and_refs;
 
diff --git a/src/object/certificate.h b/src/object/certificate.h
index 2c879d5d..023d538b 100644
--- a/src/object/certificate.h
+++ b/src/object/certificate.h
@@ -66,6 +66,6 @@ int certificate_validate_extensions_ee(X509 *, OCTET_STRING_t *,
     struct certificate_refs *, enum rpki_policy *);
 
 int certificate_traverse(struct rpp *, struct rpki_uri const *,
-    STACK_OF(X509_CRL) *, bool);
+    STACK_OF(X509_CRL) *);
 
 #endif /* SRC_OBJECT_CERTIFICATE_H_ */
diff --git a/src/object/roa.c b/src/object/roa.c
index 7de259ae..58d3783f 100644
--- a/src/object/roa.c
+++ b/src/object/roa.c
@@ -200,33 +200,32 @@ roa_traverse(struct rpki_uri const *uri, struct rpp *pp,
 
 	error = signed_object_args_init(&sobj_args, uri, crls, false);
 	if (error)
-		goto end1;
+		goto revert_fnstack;
 
 	error = signed_object_decode(&sobj_args, &arcs, roa_decode, &roa);
 	if (error)
-		goto end2;
+		goto revert_sobj;
+
+	error = refs_validate_ee(&sobj_args.refs, pp, sobj_args.uri);
+	if (error)
+		goto revert_roa;
 
 	error = vhandler_traverse_down(sobj_args.subject_name);
 	if (error)
-		goto end3;
+		goto revert_roa;
 
 	error = __handle_roa(roa, sobj_args.res);
 	if (error)
-		goto end3;
+		goto revert_roa;
 
 	error = vhandler_traverse_up();
-	if (error)
-		goto end3;
 
-	/* TODO why is this happening so late? */
-	error = refs_validate_ee(&sobj_args.refs, pp, sobj_args.uri);
-
-end3:
+revert_roa:
 	ASN_STRUCT_FREE(asn_DEF_RouteOriginAttestation, roa);
-end2:
+revert_sobj:
 	signed_object_args_cleanup(&sobj_args);
-end1:
-	pr_debug_rm("}");
+revert_fnstack:
 	fnstack_pop();
+	pr_debug_rm("}");
 	return error;
 }
diff --git a/src/object/tal.c b/src/object/tal.c
index 16a69aa8..6e98757a 100644
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -315,7 +315,7 @@ handle_tal_uri(struct tal *tal, struct rpki_uri const *uri, void *arg)
 		goto end;
 	}
 
-	error = certificate_traverse(NULL, uri, NULL, true);
+	error = certificate_traverse(NULL, uri, NULL);
 	if (error) {
 		switch (validation_pubkey_state(state)) {
 		case PKS_INVALID:
diff --git a/src/rpp.c b/src/rpp.c
index 1896355e..afed7e2d 100644
--- a/src/rpp.c
+++ b/src/rpp.c
@@ -33,26 +33,14 @@ rpp_create(void)
 
 	result = malloc(sizeof(struct rpp));
 	if (result == NULL)
-		goto fail1;
+		return NULL;
 
-	if (uris_init(&result->certs) != 0)
-		goto fail2;
+	uris_init(&result->certs);
 	result->crl_set = false;
-	if (uris_init(&result->roas) != 0)
-		goto fail3;
-	if (uris_init(&result->ghostbusters) != 0)
-		goto fail4;
+	uris_init(&result->roas);
+	uris_init(&result->ghostbusters);
 
 	return result;
-
-fail4:
-	uris_cleanup(&result->roas, uri_cleanup);
-fail3:
-	uris_cleanup(&result->certs, uri_cleanup);
-fail2:
-	free(result);
-fail1:
-	return NULL;
 }
 
 void
@@ -146,7 +134,7 @@ rpp_traverse(struct rpp *pp)
 
 	/* Use CRL stack to validate certificates, and also traverse them. */
 	ARRAYLIST_FOREACH(&pp->certs, uri)
-		certificate_traverse(pp, uri, crls, false);
+		certificate_traverse(pp, uri, crls);
 
 	/* Use valid address ranges to print ROAs that match them. */
 	ARRAYLIST_FOREACH(&pp->roas, uri)
diff --git a/src/rtr/db/delta.c b/src/rtr/db/delta.c
index 71fa960d..d53085f0 100644
--- a/src/rtr/db/delta.c
+++ b/src/rtr/db/delta.c
@@ -32,37 +32,18 @@ int
 deltas_create(struct deltas **_result)
 {
 	struct deltas *result;
-	int error;
 
 	result = malloc(sizeof(struct deltas));
 	if (result == NULL)
 		return pr_enomem();
 
-	error = deltas_v4_init(&result->v4.adds);
-	if (error)
-		goto revert_result;
-	error = deltas_v4_init(&result->v4.removes);
-	if (error)
-		goto revert_v4_adds;
-	error = deltas_v6_init(&result->v6.adds);
-	if (error)
-		goto revert_v4_removes;
-	error = deltas_v6_init(&result->v6.removes);
-	if (error)
-		goto revert_v6_adds;
+	deltas_v4_init(&result->v4.adds);
+	deltas_v4_init(&result->v4.removes);
+	deltas_v6_init(&result->v6.adds);
+	deltas_v6_init(&result->v6.removes);
 
 	*_result = result;
 	return 0;
-
-revert_v6_adds:
-	deltas_v6_cleanup(&result->v6.adds, NULL);
-revert_v4_removes:
-	deltas_v4_cleanup(&result->v4.removes, NULL);
-revert_v4_adds:
-	deltas_v4_cleanup(&result->v4.adds, NULL);
-revert_result:
-	free(result);
-	return error;
 }
 
 void
diff --git a/src/rtr/db/roa.c b/src/rtr/db/roa.c
index b40bda2e..63940f33 100644
--- a/src/rtr/db/roa.c
+++ b/src/rtr/db/roa.c
@@ -3,51 +3,28 @@
 DEFINE_ARRAY_LIST_FUNCTIONS(v4_addresses, struct v4_address)
 DEFINE_ARRAY_LIST_FUNCTIONS(v6_addresses, struct v6_address)
 
-static void
-v4_address_destroy(struct v4_address *addr)
-{
-	free(addr);
-}
-
-static void
-v6_address_destroy(struct v6_address *addr)
-{
-	free(addr);
-}
-
 int
 roa_create(u_int32_t as, struct roa **_result)
 {
 	struct roa *result;
-	int error;
 
 	result = malloc(sizeof(struct roa));
 	if (result == NULL)
 		return pr_enomem();
 
 	result->as = as;
-	error = v4_addresses_init(&result->addrs4);
-	if (error)
-		goto revert_result;
-	error = v6_addresses_init(&result->addrs6);
-	if (error)
-		goto revert_addrs4;
+	v4_addresses_init(&result->addrs4);
+	v6_addresses_init(&result->addrs6);
 
 	*_result = result;
 	return 0;
-
-revert_addrs4:
-	v4_addresses_cleanup(&result->addrs4, v4_address_destroy);
-revert_result:
-	free(result);
-	return error;
 }
 
 void
 roa_destroy(struct roa *roa)
 {
-	v4_addresses_cleanup(&roa->addrs4, v4_address_destroy);
-	v6_addresses_cleanup(&roa->addrs6, v6_address_destroy);
+	v4_addresses_cleanup(&roa->addrs4, NULL);
+	v6_addresses_cleanup(&roa->addrs6, NULL);
 }
 
 int
diff --git a/src/rtr/db/roa_tree.c b/src/rtr/db/roa_tree.c
index 86d84df7..7bd0a7b6 100644
--- a/src/rtr/db/roa_tree.c
+++ b/src/rtr/db/roa_tree.c
@@ -38,7 +38,7 @@ node_create(struct rfc5280_name *subject_name, struct node *parent)
 	node->subject_name = subject_name;
 	x509_name_get(subject_name);
 	node->parent = parent;
-	node->children.array = NULL;
+	nodes_init(&node->children);
 	node->roa = NULL;
 	return node;
 }
@@ -47,8 +47,7 @@ static void
 node_destroy(struct node *node)
 {
 	x509_name_put(node->subject_name);
-	if (node->children.array != NULL)
-		nodes_cleanup(&node->children, node_destroy);
+	nodes_cleanup(&node->children, node_destroy);
 	if (node->roa != NULL)
 		roa_destroy(node->roa);
 	free(node);
@@ -60,12 +59,6 @@ node_add_child(struct node *parent, struct rfc5280_name *subject_name)
 	struct node *child;
 	int error;
 
-	if (parent->children.array == NULL) {
-		error = nodes_init(&parent->children);
-		if (error)
-			return error;
-	}
-
 	child = node_create(subject_name, parent);
 	if (child == NULL)
 		return pr_enomem();
@@ -363,8 +356,7 @@ handle_delta_children(struct nodes *children1, struct nodes *children2,
 	 * Changes to one function might need to cascade to the other.
 	 */
 
-	struct node *c1array;
-	array_index c1; /* counter for c1array */
+	struct node *c1node;
 
 	struct node *c2array;
 	array_index c2; /* counter for c2array */
@@ -374,21 +366,20 @@ handle_delta_children(struct nodes *children1, struct nodes *children2,
 
 	int error = 0;
 
-	c1array = children1->array;
 	c2array = children2->array;
 	arridx_init(&c2indexer, children2->len);
 
-	for (c1 = 0; c1 < children1->len; c1++) {
-		if (find_subject_name(&c2indexer, c1array[c1].subject_name,
+	ARRAYLIST_FOREACH(children1, c1node) {
+		if (find_subject_name(&c2indexer, c1node->subject_name,
 		    c2array, &c2)) {
-			error = compute_deltas_node(&c1array[c1], &c2array[c2],
+			error = compute_deltas_node(c1node, &c2array[c2],
 			    deltas);
 			if (error)
 				goto end;
 
 			error = arridx_remove(&c2indexer);
 		} else {
-			error = add_all_deltas(&c1array[c1], deltas, DELTA_RM);
+			error = add_all_deltas(c1node, deltas, DELTA_RM);
 		}
 		if (error)
 			goto end;
diff --git a/src/rtr/db/roa_tree.h b/src/rtr/db/roa_tree.h
index b80c6bcb..b6b37087 100644
--- a/src/rtr/db/roa_tree.h
+++ b/src/rtr/db/roa_tree.h
@@ -16,7 +16,7 @@ void roa_tree_put(struct roa_tree *);
 
 int roa_tree_foreach_roa(struct roa_tree *, vrp_foreach_cb, void *);
 
-/* TODO rename to tree handler or whatever */
+/* TODO (urgent) rename to tree handler or whatever */
 int forthandler_reset(struct roa_tree *);
 int forthandler_go_down(struct roa_tree *, struct rfc5280_name *);
 int forthandler_go_up(struct roa_tree *);
diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c
index b94c0767..97d7cbce 100644
--- a/src/rtr/db/vrps.c
+++ b/src/rtr/db/vrps.c
@@ -41,16 +41,12 @@ delta_destroy(struct delta *delta)
 	deltas_destroy(delta->deltas);
 }
 
-int
+void
 vrps_init(void)
 {
-	int error;
-
 	state.base = NULL;
 
-	error = deltas_db_init(&state.deltas);
-	if (error)
-		return error;
+	deltas_db_init(&state.deltas);
 
 	/*
 	 * Use the same start serial, the session ID will avoid
@@ -62,17 +58,13 @@ vrps_init(void)
 	/* Get the bits that'll fit in session_id */
 	state.v0_session_id = time(NULL) & 0xFFFF;
 	/* Minus 1 to prevent same ID */
-	/*
-	 * TODO Assigning an int (and potentially negative) to a uint16_t.
-	 * Is this legal?
-	 */
-	state.v1_session_id = state.v0_session_id - 1;
+	state.v1_session_id = (state.v0_session_id != 0)
+	    ? (state.v0_session_id - 1)
+	    : (0xFFFFu);
 
 	sem_init(&rlock, 0, 1);
 	sem_init(&wlock, 0, 1);
 	rcounter = 0;
-
-	return 0;
 }
 
 void
diff --git a/src/rtr/db/vrps.h b/src/rtr/db/vrps.h
index 40cbb6b3..6b7eed6f 100644
--- a/src/rtr/db/vrps.h
+++ b/src/rtr/db/vrps.h
@@ -17,7 +17,7 @@ enum delta_status {
 	DS_DIFF_AVAILABLE,
 };
 
-int vrps_init(void);
+void vrps_init(void);
 void vrps_destroy(void);
 
 int vrps_update(struct roa_tree *, struct deltas *);
diff --git a/src/rtr/pdu_handler.c b/src/rtr/pdu_handler.c
index 6a5313e9..2b09b85e 100644
--- a/src/rtr/pdu_handler.c
+++ b/src/rtr/pdu_handler.c
@@ -47,8 +47,8 @@ send_commmon_exchange(struct sender_common *common,
 }
 
 /*
- * TODO The semaphoring is bonkers. The code keeps locking, storing a value,
- * unlocking, locking again, and using the old value.
+ * TODO (urgent) The semaphoring is bonkers. The code keeps locking, storing a
+ * value, unlocking, locking again, and using the old value.
  * It doesn't look like it's a problem for now, but eventually will be, when old
  * delta forgetting is implemented.
  * I'm going to defer this because it shouldn't be done during the merge.
diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c
index 66316b6f..7a73ec1a 100644
--- a/src/rtr/rtr.c
+++ b/src/rtr/rtr.c
@@ -19,7 +19,7 @@
 #include "rtr/err_pdu.h"
 #include "rtr/pdu.h"
 
-/* TODO Support both RTR v0 an v1 */
+/* TODO (next iteration) Support both RTR v0 an v1 */
 #define RTR_VERSION_SUPPORTED	RTR_V0
 
 volatile bool loop;
diff --git a/src/state.c b/src/state.c
index ef0be179..82825925 100644
--- a/src/state.c
+++ b/src/state.c
@@ -264,12 +264,8 @@ validation_push_cert(struct validation *state, struct rpki_uri const *cert_uri,
 	}
 
 	cert->uri = *cert_uri;
-	error = serial_numbers_init(&cert->serials);
-	if (error)
-		goto end2;
-	error = subjects_init(&cert->subjects);
-	if (error)
-		goto end3;
+	serial_numbers_init(&cert->serials);
+	subjects_init(&cert->subjects);
 	cert->resources = resources_create(false);
 	if (cert->resources == NULL) {
 		error = pr_enomem();
@@ -306,8 +302,8 @@ validation_push_cert(struct validation *state, struct rpki_uri const *cert_uri,
 
 end5:	resources_destroy(cert->resources);
 end4:	subjects_cleanup(&cert->subjects, subject_cleanup);
-end3:	serial_numbers_cleanup(&cert->serials, serial_cleanup);
-end2:	free(cert);
+	serial_numbers_cleanup(&cert->serials, serial_cleanup);
+	free(cert);
 end1:	return error;
 }
 
diff --git a/src/updates_daemon.c b/src/updates_daemon.c
index 9c07a512..621ce1fb 100644
--- a/src/updates_daemon.c
+++ b/src/updates_daemon.c
@@ -116,7 +116,7 @@ check_vrps_updates(void *param_void)
 		pr_debug("Database updated successfully. Sleeping...");
 
 sleep:
-		sleep(config_get_vrps_check_interval());
+		sleep(config_get_validation_interval());
 	} while (true);
 
 	return NULL;