From 8d38af64d1d34bff9b2548df29f3a680cdfbb0c6 Mon Sep 17 00:00:00 2001
From: Dimitri Staessens <dimitri@ouroboros.rocks>
Date: Sat, 15 Feb 2020 10:07:56 +0100
Subject: 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 <dimitri@ouroboros.rocks>
Signed-off-by: Sander Vrijders <sander@ouroboros.rocks>
---
src/ipcpd/unicast/pol/graph.c | 67 +++++++++++++++++++++++++++-----------
src/ipcpd/unicast/pol/link_state.c | 30 ++++++++++++++---
2 files changed, 74 insertions(+), 23 deletions(-)
(limited to 'src/ipcpd/unicast')
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));
--
cgit v1.2.3