Remove unintended recursion in vec_delete.

This prevents a segfault: Fixes #2225.

Add a little more documentation around the responsibilities of callers
to make this a little less likely to happen again.

I thought about calling this vec_vec_delete but went with cycles_delete;
the only place this is used is for cycles.

This effectively reverts c4ef19f483d577a8395a1a1f9ca5a321af3472b5,
and fixes that commit's memory leak in a different way.

diff --git a/lib/common/routespl.c b/lib/common/routespl.c
index 66fd79a5f6f028afa90b0dbeb0a0483e00883518..cfd9d9475358e568df6b1ed0eb0b65a316ef2f63 100644 (file)
--- a/lib/common/routespl.c
+++ b/lib/common/routespl.c
@@ -835,13 +835,18 @@ static void* vec_get(vec* pvec, size_t index)
 
 static void vec_delete(vec* pvec)
 {
-    for (size_t i = 0; i < vec_length(pvec); ++i) {
-      vec_delete(vec_get(pvec, i));
-    }
     free(pvec->_mem);
     free(pvec);
 }
 
+// cycles is assumed to be a vec of vec of nodes.
+static void cycles_delete(vec* cycles) {
+  for (size_t i = 0; i < vec_length(cycles); ++i) {
+    vec_delete(vec_get(cycles, i));
+  }
+  vec_delete(cycles);
+}
+
 static void vec_push_back(vec* pvec, void* data)
 {
     if (pvec->_elems == pvec->_capelems) {
@@ -961,6 +966,7 @@ static void dfs(graph_t *g, node_t* search, vec* visited, node_t* end, vec* cycl
        }
 }
 
+// Returns a vec of vec of nodes (aka a vector of cycles), which must be freed using cycles_delete.
 static vec* find_all_cycles(graph_t *g)
 {
     node_t *n;
@@ -976,7 +982,7 @@ static vec* find_all_cycles(graph_t *g)
                dfs(g, n, cycle, n, cycles);
        }
        
-       vec_delete(alloced_cycles); //cycles contains copied vecs
+       cycles_delete(alloced_cycles); //cycles contains copied vecs
     return cycles;
 }
 
@@ -1019,7 +1025,7 @@ static pointf get_cycle_centroid(graph_t *g, edge_t* edge)
 
        if (cycle == NULL) {
                if (cycles != NULL)
-                       vec_delete(cycles);
+                       cycles_delete(cycles);
                return get_centroid(g);
        }
 
@@ -1033,7 +1039,7 @@ static pointf get_cycle_centroid(graph_t *g, edge_t* edge)
        }
 
        if (cycles != NULL)
-               vec_delete(cycles);
+               cycles_delete(cycles);
 
        sum.x /= cnt;
     sum.y /= cnt;

diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index aae323e4007fe8a64660c00186d09aaab00fce93..aab58d34a295e5ecd51fe103e3e162a007f93004 100644 (file)
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -1741,7 +1741,6 @@ def test_gvpr_usage():
   assert "-o <ofile> - write output to <ofile>; stdout by default" in stderr, \
     "truncated or malformed GVPR usage information"
 
-@pytest.mark.xfail() # FIXME
 def test_2225():
   """
   sfdp should not segfault with curved splines