From 9689454cb2d3d3b3c9e02a32ab9db13f49e8d5a2 Mon Sep 17 00:00:00 2001 From: Dmitri Pal Date: Fri, 10 Jul 2009 11:43:05 -0400 Subject: COLLECTION Fixed: iterator_up and insert_into_current During a review of the previous patch the two issues were found: a) The col_iterator_up function was not implemented properly so it got reworked. New implementation changes the way error condition is handled. Comments were updated accordingly. b) There was a mising check for validity of the argument in the col_insert_into_current function. Check was added. c) Unit test modified to reflect the change in functionality. --- common/collection/collection.c | 18 ++++++++-- common/collection/collection.h | 6 ++-- common/collection/collection_ut.c | 76 ++++++++++++++++++++++++++++++++------- 3 files changed, 83 insertions(+), 17 deletions(-) (limited to 'common') diff --git a/common/collection/collection.c b/common/collection/collection.c index b4a2c8d5a..1e3fe5c3f 100644 --- a/common/collection/collection.c +++ b/common/collection/collection.c @@ -355,6 +355,10 @@ int col_insert_item_into_current(struct collection_item *collection, "Adding header item to new collection."); collection = item; } + else { + TRACE_ERROR_STRING("Passed in item is invalid", ""); + return EINVAL; + } } else { /* We can add items only to collections */ @@ -1243,6 +1247,9 @@ static int col_walk_items(struct collection_item *ci, TRACE_INFO_NUMBER("Mode flags:", mode_flags); /* Increase depth */ + /* NOTE: The depth is increased at the entry to the function. + * and decreased right before the exit so it is safe to decrease it. + */ (*depth)++; current = ci; @@ -2605,18 +2612,23 @@ int col_bind_iterator(struct collection_iterator **iterator, /* Stop processing this subcollection and move to the next item in the * collection 'level' levels up.*/ -int col_iterate_up(struct collection_iterator *iterator, int level) +int col_iterate_up(struct collection_iterator *iterator, unsigned level) { TRACE_FLOW_STRING("iterate_up", "Entry"); - if ((iterator == NULL) || (level >= iterator->stack_depth)) { + if (iterator == NULL) { TRACE_ERROR_NUMBER("Invalid parameter.", EINVAL); return EINVAL; } TRACE_INFO_NUMBER("Going up:", level); + TRACE_INFO_NUMBER("Current stack depth:", iterator->stack_depth); - iterator->stack_depth--; + /* If level is big just move to the top, + * that will end the iteration process. + */ + if (level >= iterator->stack_depth) iterator->stack_depth = 0; + else iterator->stack_depth -= level; TRACE_INFO_NUMBER("Stack depth at the end:", iterator->stack_depth); TRACE_FLOW_STRING("col_iterate_up", "Exit"); diff --git a/common/collection/collection.h b/common/collection/collection.h index 7de17d2e2..5483c1381 100644 --- a/common/collection/collection.h +++ b/common/collection/collection.h @@ -544,9 +544,11 @@ int col_iterate_collection(struct collection_iterator *iterator, * collection 'level' levels up. * The 'Level' parameter indicates how many levels up you want to jump. * If 0 - call is a no op. - * If the depth is less than requested level function will return error EINVAL. + * If the depth is less then requested level the iterator will + * get to the 0 level and next call to col_iterate_collection + * will return NULL item. */ -int col_iterate_up(struct collection_iterator *iterator, int level); +int col_iterate_up(struct collection_iterator *iterator, unsigned level); /* How deep are we relative to the top level. * This function might report depth that might look misleading. diff --git a/common/collection/collection_ut.c b/common/collection/collection_ut.c index c0adf348f..c50a83847 100644 --- a/common/collection/collection_ut.c +++ b/common/collection/collection_ut.c @@ -676,13 +676,14 @@ int iterator_test(void) return error; } + /* Are we done ? */ + if (item == (struct collection_item *)(NULL)) break; + depth = 0; col_get_item_depth(iterator, &depth); idepth = 0; col_get_iterator_depth(iterator, &idepth); - /* Are we done ? */ - if (item == (struct collection_item *)(NULL)) break; printf("%*sProperty (%s), type = %d, data size = %d depth = %d idepth = %d\n", depth * 4, "", @@ -695,13 +696,7 @@ int iterator_test(void) if ((strcmp(col_get_item_property(item, NULL), "id")==0) && (*((int *)(col_get_item_data(item))) == 1)) { printf("\n\nFound property we need - go up!!!\n\n\n"); - error = col_iterate_up(iterator, 5); - if (!error) { - printf("We expected error but got seucces - bad.\n"); - col_unbind_iterator(iterator); - col_destroy_collection(peer); - return -1; - } + /* This should work! */ error = col_iterate_up(iterator, 1); if (error) { @@ -925,9 +920,6 @@ int iterator_test(void) return error; } - /* This should also work becuase iterator holds to collection */ - col_destroy_collection(peer); - printf("\n\nIteration (7 show headers only no END):\n\n"); do { @@ -936,6 +928,7 @@ int iterator_test(void) error = col_iterate_collection(iterator, &item); if (error) { printf("Error (iterate): %d\n", error); + col_destroy_collection(peer); col_unbind_iterator(iterator); return error; } @@ -954,6 +947,65 @@ int iterator_test(void) /* Do not forget to unbind iterator - otherwise there will be a leak */ col_unbind_iterator(iterator); + + /* Bind iterator */ + error = col_bind_iterator(&iterator, peer, COL_TRAVERSE_DEFAULT); + if (error) { + printf("Error (bind): %d\n", error); + col_destroy_collection(peer); + return error; + } + + col_destroy_collection(peer); + + printf("\n\nIterate up test:\n\n"); + + do { + + /* Loop through a collection */ + error = col_iterate_collection(iterator, &item); + if (error) { + printf("Error (iterate): %d\n", error); + col_unbind_iterator(iterator); + return error; + } + + /* Are we done ? */ + if (item == (struct collection_item *)(NULL)) break; + + depth = 0; + col_get_item_depth(iterator, &depth); + idepth = 0; + col_get_iterator_depth(iterator, &idepth); + + + printf("%*sProperty (%s), type = %d, data size = %d depth = %d idepth = %d\n", + depth * 4, "", + col_get_item_property(item, NULL), + col_get_item_type(item), + col_get_item_length(item), + depth, + idepth); + + if (strcmp(col_get_item_property(item, NULL), "queue") == 0) { + + printf("\n\nFound property we need - go up!!!\n"); + printf("Expect bail out of collection processing.\n\n"); + + /* This should work! */ + error = col_iterate_up(iterator, 10); + if (error) { + printf("We expected success but got error %d\n", error); + col_unbind_iterator(iterator); + col_destroy_collection(peer); + return error; + } + + } + } + while(1); + + col_unbind_iterator(iterator); return EOK; } -- cgit