From 236ca39f5613e3bfce8bc067436615258ae60325 Mon Sep 17 00:00:00 2001 From: Antonov Misha Date: Tue, 23 May 2023 16:39:06 +0000 Subject: [PATCH 1/2] fix missed destroy of vector --- src/rinterface_extra.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rinterface_extra.c b/src/rinterface_extra.c index 04137a7d04d..a00ee4d858b 100644 --- a/src/rinterface_extra.c +++ b/src/rinterface_extra.c @@ -2906,6 +2906,7 @@ void R_igraph_restore_pointer(SEXP graph) { igraph_empty(&g, n, directed); igraph_add_edges(&g, &v, NULL); + igraph_vector_destroy(&v); R_igraph_set_pointer(graph, &g); } From d5efbaad1ea673a683d6739da81ef104ebede4cd Mon Sep 17 00:00:00 2001 From: Antonov548 Date: Wed, 24 May 2023 12:33:14 +0200 Subject: [PATCH 2/2] memory safety `restore_pointer` --- src/rinterface_extra.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/rinterface_extra.c b/src/rinterface_extra.c index a00ee4d858b..57f18e8b197 100644 --- a/src/rinterface_extra.c +++ b/src/rinterface_extra.c @@ -2883,11 +2883,9 @@ void R_igraph_set_pointer(SEXP result, const igraph_t* graph) { UNPROTECT(px); } -void R_igraph_restore_pointer(SEXP graph) { - igraph_t g; - igraph_vector_t v; - igraph_integer_t n=REAL(VECTOR_ELT(graph, igraph_t_idx_n))[0]; - igraph_bool_t directed=LOGICAL(VECTOR_ELT(graph, igraph_t_idx_directed))[0]; +static int restore_pointer(SEXP graph, igraph_t *g) { + igraph_integer_t no_of_nodes = REAL(VECTOR_ELT(graph, igraph_t_idx_n))[0]; + igraph_bool_t directed = LOGICAL(VECTOR_ELT(graph, igraph_t_idx_directed))[0]; igraph_vector_t from; R_SEXP_to_vector(VECTOR_ELT(graph, igraph_t_idx_from), &from); @@ -2895,18 +2893,28 @@ void R_igraph_restore_pointer(SEXP graph) { igraph_vector_t to; R_SEXP_to_vector(VECTOR_ELT(graph, igraph_t_idx_to), &to); - igraph_integer_t i, s=igraph_vector_size(&from); - igraph_vector_init(&v, s*2); + igraph_vector_t edges; + igraph_integer_t no_of_edges=igraph_vector_size(&from); + IGRAPH_VECTOR_INIT_FINALLY(&edges, no_of_edges*2); - for (i = 0; i < s; ++i) - { - igraph_vector_set(&v, i*2, VECTOR(from)[i]); - igraph_vector_set(&v, i*2+1, VECTOR(to)[i]); + for (igraph_integer_t i = 0; i < no_of_edges; ++i) { + VECTOR(edges)[2*i] = VECTOR(from)[i]; + VECTOR(edges)[2*i+1] = VECTOR(to)[i]; } - igraph_empty(&g, n, directed); - igraph_add_edges(&g, &v, NULL); - igraph_vector_destroy(&v); + IGRAPH_CHECK(igraph_empty(g, no_of_nodes, directed)); + IGRAPH_FINALLY(igraph_destroy, g); + IGRAPH_CHECK(igraph_add_edges(g, &edges, NULL)); + + igraph_vector_destroy(&edges); + IGRAPH_FINALLY_CLEAN(2); /* +1 for g */ + + return IGRAPH_SUCCESS; +} + +void R_igraph_restore_pointer(SEXP graph) { + igraph_t g; + IGRAPH_R_CHECK(restore_pointer(graph, &g)); R_igraph_set_pointer(graph, &g); }