From: Tobias Brunner Date: Mon, 28 May 2018 17:09:02 +0000 (+0200) Subject: linked-list: Order of insert_before/remove_at calls doesn't matter anymore X-Git-Tag: 5.7.0dr5~29 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2c02b025896ce6eb059d148af43bf5de80ed8d8d;p=thirdparty%2Fstrongswan.git linked-list: Order of insert_before/remove_at calls doesn't matter anymore This was quite confusing previously: While calling insert_before() and then remove_at() properly replaced the current item, calling them the other way around inserted the new item before the previous item because remove_at() changed the enumerator's position to the previous item. The behavior in corner cases (calling the methods before or after enumeration) is also changed slightly. --- diff --git a/src/libstrongswan/collections/linked_list.c b/src/libstrongswan/collections/linked_list.c index 5ad7360d6f..c7342c6d6c 100644 --- a/src/libstrongswan/collections/linked_list.c +++ b/src/libstrongswan/collections/linked_list.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2015 Tobias Brunner + * Copyright (C) 2007-2018 Tobias Brunner * Copyright (C) 2005-2006 Martin Willi * Copyright (C) 2005 Jan Hutter * HSR Hochschule fuer Technik Rapperswil @@ -111,7 +111,7 @@ struct private_enumerator_t { /** * implements enumerator interface */ - enumerator_t enumerator; + enumerator_t public; /** * associated linked list @@ -122,35 +122,19 @@ struct private_enumerator_t { * current item */ element_t *current; - - /** - * enumerator has enumerated all items - */ - bool finished; }; -METHOD(enumerator_t, enumerate, bool, - private_enumerator_t *this, va_list args) +/** + * Enumerate the current item + */ +static bool do_enumerate(private_enumerator_t *this, va_list args) { void **item; VA_ARGS_VGET(args, item); - if (this->finished) - { - return FALSE; - } if (!this->current) { - this->current = this->list->first; - } - else - { - this->current = this->current->next; - } - if (!this->current) - { - this->finished = TRUE; return FALSE; } if (item) @@ -160,28 +144,46 @@ METHOD(enumerator_t, enumerate, bool, return TRUE; } +METHOD(enumerator_t, enumerate_next, bool, + private_enumerator_t *this, va_list args) +{ + if (this->current) + { + this->current = this->current->next; + } + return do_enumerate(this, args); +} + +METHOD(enumerator_t, enumerate_current, bool, + private_enumerator_t *this, va_list args) +{ + this->public.venumerate = _enumerate_next; + return do_enumerate(this, args); +} + METHOD(linked_list_t, create_enumerator, enumerator_t*, private_linked_list_t *this) { private_enumerator_t *enumerator; INIT(enumerator, - .enumerator = { + .public = { .enumerate = enumerator_enumerate_default, - .venumerate = _enumerate, + .venumerate = _enumerate_current, .destroy = (void*)free, }, .list = this, + .current = this->first, ); - return &enumerator->enumerator; + return &enumerator->public; } METHOD(linked_list_t, reset_enumerator, void, private_linked_list_t *this, private_enumerator_t *enumerator) { - enumerator->current = NULL; - enumerator->finished = FALSE; + enumerator->current = this->first; + enumerator->public.venumerate = _enumerate_current; } METHOD(linked_list_t, get_count, int, @@ -298,14 +300,7 @@ METHOD(linked_list_t, insert_before, void, current = enumerator->current; if (!current) { - if (enumerator->finished) - { - this->public.insert_last(&this->public, item); - } - else - { - this->public.insert_first(&this->public, item); - } + insert_last(this, item); return; } element = element_create(item); @@ -377,7 +372,9 @@ METHOD(linked_list_t, remove_at, void, if (enumerator->current) { current = enumerator->current; - enumerator->current = current->previous; + enumerator->current = current->next; + /* the enumerator already points to the next item */ + enumerator->public.venumerate = _enumerate_current; remove_element(this, current); } } diff --git a/src/libstrongswan/collections/linked_list.h b/src/libstrongswan/collections/linked_list.h index a9cb7f0d45..315fb05209 100644 --- a/src/libstrongswan/collections/linked_list.h +++ b/src/libstrongswan/collections/linked_list.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2017 Tobias Brunner + * Copyright (C) 2007-2018 Tobias Brunner * Copyright (C) 2005-2008 Martin Willi * Copyright (C) 2005 Jan Hutter * HSR Hochschule fuer Technik Rapperswil @@ -102,12 +102,17 @@ struct linked_list_t { /** * Inserts a new item before the item the enumerator currently points to. * - * If this method is called before starting the enumeration the item is - * inserted first. If it is called after all items have been enumerated - * the item is inserted last. This is helpful when inserting items into - * a sorted list. + * If this method is called after all items have been enumerated, the item + * is inserted last. This is helpful when inserting items into a sorted + * list. * - * @note The position of the enumerator is not changed. + * @note The position of the enumerator is not changed. So it is safe to + * call this before or after remove_at() to replace the item at the current + * position (the enumerator will continue with the next item in the list). + * And in particular, when inserting an item before calling enumerate(), + * the enumeration will continue (or start) at the item that was first in + * the list before any items were inserted (enumerate() will return FALSE + * if the list was empty before). * * @param enumerator enumerator with position * @param item item value to insert in list @@ -118,6 +123,10 @@ struct linked_list_t { /** * Remove an item from the list where the enumerator points to. * + * If this method is called before calling enumerate() of the enumerator, + * the first item in the list, if any, will be removed. No item is removed, + * if the method is called after enumerating all items. + * * @param enumerator enumerator with position */ void (*remove_at)(linked_list_t *this, enumerator_t *enumerator); diff --git a/src/libstrongswan/tests/suites/test_linked_list_enumerator.c b/src/libstrongswan/tests/suites/test_linked_list_enumerator.c index 19f381ef3a..30b7b5c118 100644 --- a/src/libstrongswan/tests/suites/test_linked_list_enumerator.c +++ b/src/libstrongswan/tests/suites/test_linked_list_enumerator.c @@ -144,11 +144,12 @@ START_TEST(test_insert_before_ends) int round; enumerator = list->create_enumerator(list); + /* this does not change the enumerator position, which points to 1 */ list->insert_before(list, enumerator, (void*)0); ck_assert_int_eq(list->get_count(list), 6); ck_assert(list->get_first(list, (void*)&x) == SUCCESS); ck_assert_int_eq(x, 0); - round = 0; + round = 1; while (enumerator->enumerate(enumerator, &x)) { ck_assert_int_eq(round, x); @@ -177,8 +178,13 @@ START_TEST(test_insert_before_empty) ck_assert_int_eq(x, 1); ck_assert(list->get_last(list, (void*)&x) == SUCCESS); ck_assert_int_eq(x, 1); - ck_assert(enumerator->enumerate(enumerator, &x)); + ck_assert(!enumerator->enumerate(enumerator, &x)); + list->insert_before(list, enumerator, (void*)2); + ck_assert_int_eq(list->get_count(list), 2); + ck_assert(list->get_first(list, (void*)&x) == SUCCESS); ck_assert_int_eq(x, 1); + ck_assert(list->get_last(list, (void*)&x) == SUCCESS); + ck_assert_int_eq(x, 2); ck_assert(!enumerator->enumerate(enumerator, NULL)); enumerator->destroy(enumerator); } @@ -221,6 +227,43 @@ START_TEST(test_remove_at) } END_TEST +START_TEST(test_remove_at_multi) +{ + enumerator_t *enumerator; + intptr_t x; + int round; + + round = 1; + enumerator = list->create_enumerator(list); + while (enumerator->enumerate(enumerator, &x)) + { + ck_assert_int_eq(round, x); + if (round == 2 || round == 5) + { + list->remove_at(list, enumerator); + } + round++; + } + ck_assert_int_eq(list->get_count(list), 3); + list->reset_enumerator(list, enumerator); + round = 1; + while (enumerator->enumerate(enumerator, &x)) + { + if (round == 2) + { /* skip removed item */ + round++; + } + ck_assert_int_eq(round, x); + list->remove_at(list, enumerator); + round++; + } + ck_assert_int_eq(list->get_count(list), 0); + list->reset_enumerator(list, enumerator); + ck_assert(!enumerator->enumerate(enumerator, &x)); + enumerator->destroy(enumerator); +} +END_TEST + START_TEST(test_remove_at_ends) { enumerator_t *enumerator; @@ -228,14 +271,14 @@ START_TEST(test_remove_at_ends) enumerator = list->create_enumerator(list); list->remove_at(list, enumerator); - ck_assert_int_eq(list->get_count(list), 5); + ck_assert_int_eq(list->get_count(list), 4); ck_assert(list->get_first(list, (void*)&x) == SUCCESS); - ck_assert_int_eq(x, 1); + ck_assert_int_eq(x, 2); while (enumerator->enumerate(enumerator, &x)) { } list->remove_at(list, enumerator); - ck_assert_int_eq(list->get_count(list), 5); + ck_assert_int_eq(list->get_count(list), 4); ck_assert(list->get_last(list, (void*)&x) == SUCCESS); ck_assert_int_eq(x, 5); enumerator->destroy(enumerator); @@ -254,14 +297,12 @@ START_TEST(test_insert_before_remove_at) { ck_assert_int_eq(round, x); if (round == 2) - { /* this replaces the current item, as insert_before does not change - * the enumerator position */ + { /* this replaces the current item */ list->insert_before(list, enumerator, (void*)42); list->remove_at(list, enumerator); } else if (round == 4) - { /* this does not replace the item, as remove_at moves the enumerator - * position to the previous item */ + { /* same here, the order of calls does not matter */ list->remove_at(list, enumerator); list->insert_before(list, enumerator, (void*)21); } @@ -276,13 +317,9 @@ START_TEST(test_insert_before_remove_at) { /* check replaced item */ ck_assert_int_eq(42, x); } - else if (round == 3) - { /* check misplaced item */ - ck_assert_int_eq(21, x); - } else if (round == 4) - { /* check misplaced item */ - ck_assert_int_eq(3, x); + { /* check replace item */ + ck_assert_int_eq(21, x); } else { @@ -348,6 +385,7 @@ Suite *linked_list_enumerator_suite_create() tc = tcase_create("modify"); tcase_add_checked_fixture(tc, setup_list, teardown_list); tcase_add_test(tc, test_remove_at); + tcase_add_test(tc, test_remove_at_multi); tcase_add_test(tc, test_remove_at_ends); tcase_add_test(tc, test_insert_before_remove_at); suite_add_tcase(s, tc);