diff --git a/libisofs/node.c b/libisofs/node.c index 96993ba..9ba1c66 100644 --- a/libisofs/node.c +++ b/libisofs/node.c @@ -366,8 +366,9 @@ int iso_dir_get_children(const IsoDir *dir, IsoDirIter **iter) return ISO_OUT_OF_MEM; } - it->dir = dir; - it->pos = dir->children; + it->dir = (IsoDir*)dir; + it->pos = NULL; + it->flag = 0x00; *iter = it; return ISO_SUCCESS; @@ -375,21 +376,47 @@ int iso_dir_get_children(const IsoDir *dir, IsoDirIter **iter) int iso_dir_iter_next(IsoDirIter *iter, IsoNode **node) { - IsoNode *n; if (iter == NULL || node == NULL) { return ISO_NULL_POINTER; } - n = iter->pos; - if (n == NULL) { - *node = NULL; - return 0; + + /* clear next flag */ + iter->flag &= ~0x01; + + if (iter->pos == NULL) { + /* we are at the beginning */ + iter->pos = iter->dir->children; + if (iter->pos == NULL) { + /* empty dir */ + *node = NULL; + return 0; + } + } else { + if (iter->pos->parent != iter->dir) { + /* this can happen if the node has been moved to another dir */ + /* TODO specific error */ + return ISO_ERROR; + } + if (iter->pos->next == NULL) { + /* no more children */ + *node = NULL; + return 0; + } else { + /* free reference to current position */ + iso_node_unref(iter->pos); /* it is never last ref!! */ + + /* advance a position */ + iter->pos = iter->pos->next; + } } - if (n->parent != iter->dir) { - /* this can happen if the node has been moved to another dir */ - return ISO_ERROR; - } - *node = n; - iter->pos = n->next; + + /* ok, take a ref to the current position, to prevent internal errors + * if deleted somewhere */ + iso_node_ref(iter->pos); + iter->flag |= 0x01; /* set next flag */ + + /* return pointed node */ + *node = iter->pos; return ISO_SUCCESS; } @@ -406,11 +433,18 @@ int iso_dir_iter_has_next(IsoDirIter *iter) if (iter == NULL) { return ISO_NULL_POINTER; } - return iter->pos == NULL ? 0 : 1; + if (iter->pos == NULL) { + return iter->dir->children == NULL ? 0 : 1; + } else { + return iter->pos->next == NULL ? 0 : 1; + } } void iso_dir_iter_free(IsoDirIter *iter) { + if (iter->pos != NULL) { + iso_node_unref(iter->pos); + } free(iter); } @@ -448,7 +482,7 @@ int iso_node_take(IsoNode *node) pos = iso_dir_find_node(dir, node); if (pos == NULL) { /* should never occur */ - return ISO_ERROR; + return ISO_ASSERT_FAILURE; } *pos = node->next; node->parent = NULL; @@ -494,41 +528,75 @@ IsoDir *iso_node_get_parent(IsoNode *node) /* TODO #00005 optimize iso_dir_iter_take */ int iso_dir_iter_take(IsoDirIter *iter) { - IsoNode *pos; + IsoNode *pos, *pre; if (iter == NULL) { return ISO_NULL_POINTER; } - - pos = iter->dir->children; - if (iter->pos == pos) { - return ISO_ERROR; + + if (!(iter->flag & 0x01)) { + return ISO_ERROR; /* next not called or end of dir */ } - while (pos != NULL && pos->next != iter->pos) { + + if (iter->pos == NULL) { + return ISO_ASSERT_FAILURE; + } + + /* clear next flag */ + iter->flag &= ~0x01; + + pos = iter->dir->children; + pre = NULL; + while (pos != NULL && pos != iter->pos) { + pre = pos; pos = pos->next; } if (pos == NULL) { - return ISO_ERROR; + return ISO_ERROR; /* node not in dir */ } - return iso_node_take(pos); + + if (pos != iter->pos) { + return ISO_ASSERT_FAILURE; + } + + /* dispose iterator reference */ + iso_node_unref(iter->pos); + + if (pre == NULL) { + /* node is a first position */ + iter->dir->children = pos->next; + iter->pos = NULL; + } else { + pre->next = pos->next; + iter->pos = pre; + iso_node_ref(pre); /* take iter ref */ + } + + /* take pos */ + pos->parent = NULL; + pos->next = NULL; + iter->dir->nchildren--; + return ISO_SUCCESS; } int iso_dir_iter_remove(IsoDirIter *iter) { + int ret; IsoNode *pos; + if (iter == NULL) { return ISO_NULL_POINTER; } - pos = iter->dir->children; + pos = iter->pos; + + ret = iso_dir_iter_take(iter); + if (ret == ISO_SUCCESS) { + /* remove node */ + iso_node_unref(pos); + } if (iter->pos == pos) { return ISO_ERROR; } - while (pos != NULL && pos->next != iter->pos) { - pos = pos->next; - } - if (pos == NULL) { - return ISO_ERROR; - } - return iso_node_remove(pos); + return ret; } /** diff --git a/libisofs/node.h b/libisofs/node.h index b2764f9..0a77169 100644 --- a/libisofs/node.h +++ b/libisofs/node.h @@ -153,8 +153,15 @@ struct Iso_Special */ struct Iso_Dir_Iter { - const IsoDir *dir; + IsoDir *dir; + + /* points to the last visited child, to NULL before start */ IsoNode *pos; + + /* Some control flags. + * bit 0 -> 1 if next called, 0 reseted at start or on deletion + */ + int flag; }; int iso_node_new_root(IsoDir **root); diff --git a/test/test_node.c b/test/test_node.c index 8b59fb6..7c05d8f 100644 --- a/test/test_node.c +++ b/test/test_node.c @@ -410,6 +410,7 @@ void test_iso_dir_get_node() free(dir); } +static void test_iso_dir_get_children() { int result; @@ -436,6 +437,7 @@ void test_iso_dir_get_children() /* 1st node to be added */ node1 = calloc(1, sizeof(IsoNode)); node1->name = "Node1"; + node1->refcount = 1; result = iso_dir_add_node(dir, node1, 0); CU_ASSERT_EQUAL(dir->nchildren, 1); @@ -461,6 +463,7 @@ void test_iso_dir_get_children() /* add another node */ node2 = calloc(1, sizeof(IsoNode)); node2->name = "A node to be added first"; + node2->refcount = 1; result = iso_dir_add_node(dir, node2, 0); CU_ASSERT_EQUAL(result, 2); @@ -490,6 +493,7 @@ void test_iso_dir_get_children() /* addition of a 3rd node, to be inserted last */ node3 = calloc(1, sizeof(IsoNode)); node3->name = "This node will be inserted last"; + node3->refcount = 1; result = iso_dir_add_node(dir, node3, 0); CU_ASSERT_EQUAL(result, 3); @@ -520,13 +524,18 @@ void test_iso_dir_get_children() CU_ASSERT_EQUAL(result, 0); CU_ASSERT_PTR_NULL(node); iso_dir_iter_free(iter); - + + /* assert correct refcount */ + CU_ASSERT_EQUAL(node1->refcount, 1); free(node1); + CU_ASSERT_EQUAL(node2->refcount, 1); free(node2); + CU_ASSERT_EQUAL(node3->refcount, 1); free(node3); free(dir); } +static void test_iso_dir_iter_take() { int result; @@ -551,6 +560,7 @@ void test_iso_dir_iter_take() /* 1st node to be added */ node1 = calloc(1, sizeof(IsoNode)); node1->name = "Node1"; + node1->refcount = 1; result = iso_dir_add_node(dir, node1, 0); CU_ASSERT_EQUAL(dir->nchildren, 1); @@ -563,7 +573,7 @@ void test_iso_dir_iter_take() CU_ASSERT_TRUE(result < 0); /* should fail */ result = iso_dir_iter_next(iter, &node); - + /* this should remove the child */ result = iso_dir_iter_take(iter); CU_ASSERT_EQUAL(result, 1); @@ -581,6 +591,7 @@ void test_iso_dir_iter_take() node2 = calloc(1, sizeof(IsoNode)); node2->name = "A node to be added first"; + node2->refcount = 1; result = iso_dir_add_node(dir, node2, 0); CU_ASSERT_EQUAL(result, 2); @@ -658,6 +669,7 @@ void test_iso_dir_iter_take() * let's insert a node after node2 and before node1 */ node3 = calloc(1, sizeof(IsoNode)); node3->name = "A node to be added second"; + node3->refcount = 1; result = iso_dir_add_node(dir, node3, 0); CU_ASSERT_EQUAL(dir->nchildren, 3); @@ -669,13 +681,178 @@ void test_iso_dir_iter_take() CU_ASSERT_PTR_EQUAL(node3->next, node1); iso_dir_iter_free(iter); - + + /* assert correct refcount */ + CU_ASSERT_EQUAL(node1->refcount, 1); free(node1); + CU_ASSERT_EQUAL(node2->refcount, 1); free(node2); + CU_ASSERT_EQUAL(node3->refcount, 1); free(node3); free(dir); } +static +void test_iso_dir_iter_remove() +{ + int result; + IsoDirIter *iter; + IsoDir *dir; + IsoNode *node, *node1, *node2, *node3; + + /* init dir with default values, not all field need to be initialized */ + dir = malloc(sizeof(IsoDir)); + dir->children = NULL; + dir->nchildren = 0; + + result = iso_dir_get_children(dir, &iter); + CU_ASSERT_EQUAL(result, 1); + + /* remove on empty dir! */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_TRUE(result < 0); /* should fail */ + + iso_dir_iter_free(iter); + + /* 1st node to be added */ + node1 = calloc(1, sizeof(IsoNode)); + node1->name = "Node1"; + node1->refcount = 2; + result = iso_dir_add_node(dir, node1, 0); + CU_ASSERT_EQUAL(dir->nchildren, 1); + + /* test iteration again */ + result = iso_dir_get_children(dir, &iter); + CU_ASSERT_EQUAL(result, 1); + + /* remove before iso_dir_iter_next() */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_TRUE(result < 0); /* should fail */ + + result = iso_dir_iter_next(iter, &node); + + /* this should remove the child */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_EQUAL(dir->nchildren, 0); + CU_ASSERT_PTR_NULL(dir->children); + + result = iso_dir_iter_has_next(iter); + CU_ASSERT_EQUAL(result, 0); + + iso_dir_iter_free(iter); + + /* add two node */ + node1->refcount++; /* was removed above */ + result = iso_dir_add_node(dir, node1, 0); + CU_ASSERT_EQUAL(dir->nchildren, 1); + + node2 = calloc(1, sizeof(IsoNode)); + node2->name = "A node to be added first"; + node2->refcount = 2; + result = iso_dir_add_node(dir, node2, 0); + CU_ASSERT_EQUAL(result, 2); + + result = iso_dir_get_children(dir, &iter); + CU_ASSERT_EQUAL(result, 1); + + /* remove before iso_dir_iter_next() */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_TRUE(result < 0); /* should fail */ + + /* iter should have two items... */ + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_PTR_EQUAL(node, node2); + + /* remove node 2 */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_EQUAL(dir->nchildren, 1); + CU_ASSERT_PTR_EQUAL(dir->children, node1); + + /* next should still work */ + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_PTR_EQUAL(node, node1); + + /* ...and no more */ + result = iso_dir_iter_has_next(iter); + CU_ASSERT_EQUAL(result, 0); + + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 0); + CU_ASSERT_PTR_NULL(node); + iso_dir_iter_free(iter); + + /* now remove only last child */ + node2->refcount++; /* was removed above */ + result = iso_dir_add_node(dir, node2, 0); + CU_ASSERT_EQUAL(result, 2); + + result = iso_dir_get_children(dir, &iter); + CU_ASSERT_EQUAL(result, 1); + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_PTR_EQUAL(node, node2); + + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_PTR_EQUAL(node, node1); + + /* take last child */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_EQUAL(dir->nchildren, 1); + CU_ASSERT_PTR_EQUAL(dir->children, node2); + + result = iso_dir_iter_has_next(iter); + CU_ASSERT_EQUAL(result, 0); + + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 0); + CU_ASSERT_PTR_NULL(node); + iso_dir_iter_free(iter); + + /* Ok, now another situation. Modification of dir during iteration */ + node1->refcount++; + result = iso_dir_add_node(dir, node1, 0); + CU_ASSERT_EQUAL(result, 2); + + result = iso_dir_get_children(dir, &iter); + CU_ASSERT_EQUAL(result, 1); + result = iso_dir_iter_next(iter, &node); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_PTR_EQUAL(node, node2); + + /* returned dir is node2, it should be the node taken next, but + * let's insert a node after node2 and before node1 */ + node3 = calloc(1, sizeof(IsoNode)); + node3->name = "A node to be added second"; + node3->refcount = 2; + result = iso_dir_add_node(dir, node3, 0); + CU_ASSERT_EQUAL(dir->nchildren, 3); + + /* is the node 2 the removed one? */ + result = iso_dir_iter_remove(iter); + CU_ASSERT_EQUAL(result, 1); + CU_ASSERT_EQUAL(dir->nchildren, 2); + CU_ASSERT_PTR_EQUAL(dir->children, node3); + CU_ASSERT_PTR_EQUAL(node3->next, node1); + + iso_dir_iter_free(iter); + + /* assert correct refcount */ + CU_ASSERT_EQUAL(node1->refcount, 2); /* node1 is not removed */ + free(node1); + CU_ASSERT_EQUAL(node2->refcount, 1); + free(node2); + CU_ASSERT_EQUAL(node3->refcount, 2); /* node3 is not removed */ + free(node3); + free(dir); +} + +static void test_iso_node_take() { int result; @@ -690,6 +867,7 @@ void test_iso_node_take() /* 1st node to be added */ node1 = calloc(1, sizeof(IsoNode)); node1->name = "Node1"; + node1->refcount = 1; /* addition of node to an empty dir */ result = iso_dir_add_node(dir, node1, 0); @@ -710,6 +888,7 @@ void test_iso_node_take() /* addition of a 2nd node, to be inserted before */ node2 = calloc(1, sizeof(IsoNode)); node2->name = "A node to be added first"; + node2->refcount = 1; result = iso_dir_add_node(dir, node2, 0); CU_ASSERT_EQUAL(result, 2); @@ -744,6 +923,7 @@ void test_iso_node_take() /* ...and a 3rd child, to be inserted last */ node3 = calloc(1, sizeof(IsoNode)); node3->name = "This node will be inserted last"; + node3->refcount = 1; result = iso_dir_add_node(dir, node3, 0); CU_ASSERT_EQUAL(result, 3); @@ -758,9 +938,13 @@ void test_iso_node_take() CU_ASSERT_PTR_EQUAL(node3->parent, dir); CU_ASSERT_PTR_NULL(node1->next); CU_ASSERT_PTR_NULL(node1->parent); - + + /* assert correct refcount */ + CU_ASSERT_EQUAL(node1->refcount, 1); free(node1); + CU_ASSERT_EQUAL(node2->refcount, 1); free(node2); + CU_ASSERT_EQUAL(node3->refcount, 1); free(node3); free(dir); } @@ -835,6 +1019,7 @@ void add_node_suite() CU_add_test(pSuite, "iso_dir_get_node()", test_iso_dir_get_node); CU_add_test(pSuite, "iso_dir_get_children()", test_iso_dir_get_children); CU_add_test(pSuite, "iso_dir_iter_take()", test_iso_dir_iter_take); + CU_add_test(pSuite, "iso_dir_iter_remove()", test_iso_dir_iter_remove); CU_add_test(pSuite, "iso_node_take()", test_iso_node_take); CU_add_test(pSuite, "iso_node_set_name()", test_iso_node_set_name); }