lib: graph: fix deletions

Iterating over an array while deleting items needs to consider
interactions between the iteration position and deletion.  The previous
code completely ignored that problem, leading to memleaks (graph_delete
skipping half of the nodes) and dangling pointers (if parallel edges
exist in graph_remove_edge).

Iterating backwards is safe and reduces "move to fill hole" overhead in
deletion.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Cc: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/lib/graph.c b/lib/graph.c
index 891ecc33c0da8907a1245334cdf2432b8ab38026..035e552d0895d35aae79b01b30d0b6ae34f6ca48 100644 (file)
--- a/lib/graph.c
+++ b/lib/graph.c
@@ -117,11 +117,11 @@ void
 graph_remove_edge (struct graph_node *from, struct graph_node *to)
 {
   // remove from from to->from
-  for (unsigned int i = 0; i < vector_active (to->from); i++)
+  for (unsigned int i = vector_active (to->from); i--; /**/)
     if (vector_slot (to->from, i) == from)
       vector_remove (to->from, i);
   // remove to from from->to
-  for (unsigned int i = 0; i < vector_active (from->to); i++)
+  for (unsigned int i = vector_active (from->to); i--; /**/)
     if (vector_slot (from->to, i) == to)
       vector_remove (from->to, i);
 }
@@ -130,7 +130,7 @@ void
 graph_delete_graph (struct graph *graph)
 {
   // delete each node in the graph
-  for (unsigned int i = 0; i < vector_active (graph->nodes); i++)
+  for (unsigned int i = vector_active (graph->nodes); i--; /**/)
     graph_delete_node (graph, vector_slot (graph->nodes, i));
 
   vector_free (graph->nodes);