From 8d38af64d1d34bff9b2548df29f3a680cdfbb0c6 Mon Sep 17 00:00:00 2001 From: Dimitri Staessens 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 Signed-off-by: Sander Vrijders --- 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 #include -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