ipcpd: Fix minor things in graph and link_state

The vertex was used before definition in the graph
implementation. Fixed potential data race in link_state
algorithm. Added missing asserts. Removed initialization of variables
where not needed to let compiler warn about uninitialized uses.

Signed-off-by: Dimitri Staessens <[email protected]>
Signed-off-by: Sander Vrijders <[email protected]>

2 files changed, 74 insertions, 23 deletions

diff --git a/src/ipcpd/unicast/pol/graph.c b/src/ipcpd/unicast/pol/graph.c
index d5c1a9e9..d774e845 100644
--- a/src/ipcpd/unicast/pol/graph.c
+++ b/src/ipcpd/unicast/pol/graph.c
@@ -41,13 +41,6 @@
 #include <limits.h>
 #include <string.h>
 
-struct edge {
-        struct list_head next;
-        struct vertex *  nb;
-        qosspec_t        qs;
-        int              announced;
-};
-
 struct vertex {
         struct list_head next;
         uint64_t         addr;
@@ -55,6 +48,13 @@ struct vertex {
         int              index;
 };
 
+struct edge {
+        struct list_head next;
+        struct vertex *  nb;
+        qosspec_t        qs;
+        int              announced;
+};
+
 struct graph {
         size_t           nr_vertices;
         struct list_head vertices;
@@ -64,7 +64,9 @@ struct graph {
 static struct edge * find_edge_by_addr(struct vertex * vertex,
                                        uint64_t        dst_addr)
 {
-        struct list_head * p = NULL;
+        struct list_head * p;
+
+        assert(vertex);
 
         list_for_each(p, &vertex->edges) {
                 struct edge * e = list_entry(p, struct edge, next);
@@ -78,7 +80,9 @@ static struct edge * find_edge_by_addr(struct vertex * vertex,
 static struct vertex * find_vertex_by_addr(struct graph * graph,
                                            uint64_t       addr)
 {
-        struct list_head * p = NULL;
+        struct list_head * p;
+
+        assert(graph);
 
         list_for_each(p, &graph->vertices) {
                 struct vertex * e = list_entry(p, struct vertex, next);
@@ -94,11 +98,13 @@ static struct edge * add_edge(struct vertex * vertex,
 {
         struct edge * edge;
 
+        assert(vertex);
+        assert(nb);
+
         edge = malloc(sizeof(*edge));
         if (edge == NULL)
                 return NULL;
 
-        list_head_init(&edge->next);
         edge->nb = nb;
         edge->announced = 0;
 
@@ -109,8 +115,10 @@ static struct edge * add_edge(struct vertex * vertex,
 
 static void del_edge(struct edge * edge)
 {
-       list_del(&edge->next);
-       free(edge);
+        assert(edge);
+
+        list_del(&edge->next);
+        free(edge);
 }
 
 static struct vertex * add_vertex(struct graph * graph,
@@ -120,11 +128,12 @@ static struct vertex * add_vertex(struct graph * graph,
         struct list_head * p;
         int                i = 0;
 
+        assert(graph);
+
         vertex = malloc(sizeof(*vertex));
         if (vertex == NULL)
                 return NULL;
 
-        list_head_init(&vertex->next);
         list_head_init(&vertex->edges);
         vertex->addr = addr;
 
@@ -152,11 +161,14 @@ static struct vertex * add_vertex(struct graph * graph,
         return vertex;
 }
 
-static void del_vertex(struct graph * graph,
+static void del_vertex(struct graph *  graph,
                        struct vertex * vertex)
 {
-        struct list_head * p = NULL;
-        struct list_head * n = NULL;
+        struct list_head * p;
+        struct list_head * h;
+
+        assert(graph);
+        assert(vertex);
 
         list_del(&vertex->next);
 
@@ -167,7 +179,7 @@ static void del_vertex(struct graph * graph,
                         v->index--;
         }
 
-        list_for_each_safe(p, n, &vertex->edges) {
+        list_for_each_safe(p, h, &vertex->edges) {
                 struct edge * e = list_entry(p, struct edge, next);
                 del_edge(e);
         }
@@ -359,7 +371,12 @@ static int get_min_vertex(struct graph *   graph,
         int                min = INT_MAX;
         int                index = -1;
         int                i = 0;
-        struct list_head * p = NULL;
+        struct list_head * p;
+
+        assert(v);
+        assert(graph);
+        assert(dist);
+        assert(used);
 
         *v = NULL;
 
@@ -391,6 +408,10 @@ static int dijkstra(struct graph *    graph,
         struct edge *      e = NULL;
         int                alt;
 
+        assert(graph);
+        assert(nhops);
+        assert(dist);
+
         *nhops = malloc(sizeof(**nhops) * graph->nr_vertices);
         if (*nhops == NULL)
                 goto fail_pnhops;
@@ -460,6 +481,8 @@ static void free_routing_table(struct list_head * table)
         struct list_head * q;
         struct list_head * i;
 
+        assert(table);
+
         list_for_each_safe(p, h, table) {
                 struct routing_table * t =
                         list_entry(p, struct routing_table, next);
@@ -498,6 +521,10 @@ static int graph_routing_table_simple(struct graph *     graph,
         struct routing_table * t;
         struct nhop *          n;
 
+        assert(graph);
+        assert(table);
+        assert(dist);
+
         /* We need at least 2 vertices for a table */
         if (graph->nr_vertices < 2)
                 goto fail_vertices;
@@ -555,9 +582,11 @@ static int add_lfa_to_table(struct list_head * table,
                             uint64_t           addr,
                             uint64_t           lfa)
 {
-        struct list_head * p = NULL;
+        struct list_head * p;
         struct nhop *      n;
 
+        assert(table);
+
         n = malloc(sizeof(*n));
         if (n == NULL)
                 return -1;
diff --git a/src/ipcpd/unicast/pol/link_state.c b/src/ipcpd/unicast/pol/link_state.c
index 4ddebd51..e2656a98 100644
--- a/src/ipcpd/unicast/pol/link_state.c
+++ b/src/ipcpd/unicast/pol/link_state.c
@@ -144,6 +144,8 @@ static int str_adj(struct adjacency * adj,
         char        seqnobuf[64];
         struct tm * tm;
 
+        assert(adj);
+
         if (len < LS_ENTRY_SIZE)
                 return -1;
 
@@ -183,6 +185,9 @@ static int lsdb_getattr(const char *  path,
         struct adjacency * adj;
         struct timespec    now;
 
+        assert(path);
+        assert(st);
+
         clock_gettime(CLOCK_REALTIME_COARSE, &now);
 
         pthread_rwlock_rdlock(&ls.db_lock);
@@ -213,6 +218,8 @@ static int lsdb_read(const char * path,
         struct adjacency * a;
         int                size;
 
+        assert(path);
+
         pthread_rwlock_rdlock(&ls.db_lock);
 
         if (ls.db_len + ls.nbs_len == 0)
@@ -240,6 +247,8 @@ static int lsdb_readdir(char *** buf)
         char               entry[RIB_PATH_LEN + 1];
         ssize_t            idx = 0;
 
+        assert(buf);
+
         pthread_rwlock_rdlock(&ls.db_lock);
 
         if (ls.db_len + ls.nbs_len == 0) {
@@ -348,8 +357,8 @@ static int lsdb_add_nb(uint64_t     addr,
         return 0;
 }
 
-static int lsdb_del_nb(uint64_t     addr,
-                       int          fd)
+static int lsdb_del_nb(uint64_t addr,
+                       int      fd)
 {
         struct list_head * p;
         struct list_head * h;
@@ -377,14 +386,16 @@ static int lsdb_del_nb(uint64_t     addr,
 static int nbr_to_fd(uint64_t addr)
 {
         struct list_head * p;
+        int                fd;
 
         pthread_rwlock_rdlock(&ls.db_lock);
 
         list_for_each(p, &ls.nbs) {
                 struct nb * nb = list_entry(p, struct nb, next);
                 if (nb->addr == addr && nb->type == NB_DT) {
+                        fd = nb->fd;
                         pthread_rwlock_unlock(&ls.db_lock);
-                        return nb->fd;
+                        return fd;
                 }
         }
 
@@ -401,6 +412,8 @@ static void calculate_pff(struct routing_i * instance)
         struct list_head * q;
         int                fds[PROG_MAX_FLOWS];
 
+        assert(instance);
+
         if (graph_routing_table(ls.graph, ls.routing_algo,
                                 ipcpi.dt_addr, &table))
                 return;
@@ -460,6 +473,8 @@ static int lsdb_add_link(uint64_t    src,
         struct timespec    now;
         int                ret = -1;
 
+        assert(qs);
+
         clock_gettime(CLOCK_REALTIME_COARSE, &now);
 
         pthread_rwlock_wrlock(&ls.db_lock);
@@ -537,7 +552,11 @@ static int lsdb_del_link(uint64_t src,
 static void * periodic_recalc_pff(void * o)
 {
         bool               modified;
-        struct routing_i * inst = (struct routing_i *) o;
+        struct routing_i * inst;
+
+        assert(o);
+
+        inst = (struct routing_i *) o;
 
         while (true) {
                 pthread_mutex_lock(&inst->lock);
@@ -547,6 +566,7 @@ static void * periodic_recalc_pff(void * o)
 
                 if (modified)
                         calculate_pff(inst);
+
                 sleep(RECALC_TIME);
         }
 
@@ -783,6 +803,8 @@ static void handle_event(void *       self,
 
         (void) self;
 
+        assert(o);
+
         c = (struct conn *) o;
 
         memset(&qs, 0, sizeof(qs));