diff --git a/src/augeas.c b/src/augeas.c index 5bb1418fc..8c288e157 100644 --- a/src/augeas.c +++ b/src/augeas.c @@ -1142,6 +1142,7 @@ int tree_rm(struct pathx *p) { struct tree *tree, **del; int cnt = 0, ndel = 0, i; + /* set ndel to the number of trees we could possibly delete */ for (tree = pathx_first(p); tree != NULL; tree = pathx_next(p)) { if (! TREE_HIDDEN(tree)) ndel += 1; @@ -1159,12 +1160,32 @@ int tree_rm(struct pathx *p) { if (TREE_HIDDEN(tree)) continue; pathx_symtab_remove_descendants(pathx_get_symtab(p), tree); - del[i] = tree; - i += 1; + /* Collect the tree nodes that actually need to be deleted in + del. Mark the root of every subtree we are going to free by + setting tree->added. Only add a node to del if none of its + ancestors would have been freed by the time we get to freeing + that node; this avoids double frees for situations where the + path expression matches both /node and /node/child as unlinking + /node implicitly unlinks /node/child */ + int live = 1; + for (struct tree *t = tree; live && ! ROOT_P(t); t = t->parent) { + if (t->added) + live = 0; + } + if (live) { + del[i] = tree; + i += 1; + tree->added = 1; + } } + /* ndel now means: the number of trees we are actually going to delete */ + ndel = i; - for (i = 0; i < ndel; i++) - cnt += tree_unlink_raw(del[i]); + for (i = 0; i < ndel; i++) { + if (del[i] != NULL) { + cnt += tree_unlink_raw(del[i]); + } + } free(del); return cnt; diff --git a/src/internal.h b/src/internal.h index 6074b0a33..8cd530ef4 100644 --- a/src/internal.h +++ b/src/internal.h @@ -378,7 +378,8 @@ struct tree { struct tree *children; /* List of children through NEXT */ char *value; int dirty; - uint8_t added; /* only used by ns_add to dedupe nodesets */ + uint8_t added; /* only used by ns_add and tree_rm to dedupe + nodesets */ struct span *span; }; diff --git a/tests/test-api.c b/tests/test-api.c index 7b9b04250..662cdc26e 100644 --- a/tests/test-api.c +++ b/tests/test-api.c @@ -642,6 +642,20 @@ static void testAugEscape(CuTest *tc) { free(out); } +static void testRm(CuTest *tc) { + struct augeas *aug; + int r; + + aug = aug_init(root, loadpath, AUG_NO_STDINC|AUG_NO_MODL_AUTOLOAD); + CuAssertPtrNotNull(tc, aug); + + r = aug_set(aug, "/files/1/2/3/4/5", "1"); + CuAssertRetSuccess(tc, r); + + r = aug_rm(aug, "/files//*"); + CuAssertIntEquals(tc, 5, r); +} + int main(void) { char *output = NULL; CuSuite* suite = CuSuiteNew(); @@ -661,6 +675,7 @@ int main(void) { SUITE_ADD_TEST(suite, testTextStore); SUITE_ADD_TEST(suite, testTextRetrieve); SUITE_ADD_TEST(suite, testAugEscape); + SUITE_ADD_TEST(suite, testRm); abs_top_srcdir = getenv("abs_top_srcdir"); if (abs_top_srcdir == NULL)