From ec897a868cd72f4b1ac53b3a2934800ed09d580a Mon Sep 17 00:00:00 2001 From: Kaspar Schleiser Date: Wed, 13 Jul 2016 00:55:37 +0200 Subject: [PATCH 1/3] core, gnrc_netreg: remove redundant or unneeded clist.h include --- core/include/thread.h | 1 - sys/net/gnrc/netreg/gnrc_netreg.c | 1 - 2 files changed, 2 deletions(-) diff --git a/core/include/thread.h b/core/include/thread.h index 88828188b0..366e6f2edf 100644 --- a/core/include/thread.h +++ b/core/include/thread.h @@ -28,7 +28,6 @@ #include "arch/thread_arch.h" #include "cpu_conf.h" #include "sched.h" -#include "clist.h" #ifdef MODULE_CORE_THREAD_FLAGS #include "thread_flags.h" diff --git a/sys/net/gnrc/netreg/gnrc_netreg.c b/sys/net/gnrc/netreg/gnrc_netreg.c index 495b063157..7effd16f12 100644 --- a/sys/net/gnrc/netreg/gnrc_netreg.c +++ b/sys/net/gnrc/netreg/gnrc_netreg.c @@ -16,7 +16,6 @@ #include #include "assert.h" -#include "clist.h" #include "utlist.h" #include "net/gnrc/netreg.h" #include "net/gnrc/nettype.h" From 6d12a9166aea4c2985f009c1645d6a0a624eb661 Mon Sep 17 00:00:00 2001 From: Kaspar Schleiser Date: Wed, 13 Jul 2016 00:51:41 +0200 Subject: [PATCH 2/3] core: clist: API enhancements - renamed clist_insert() -> clist_rpush() - renamed clist_remove_head() -> clist_lpop() - renamed clist_advance() -> clist_lpoprpush() - added clist_lpush(), clist_rpop(), clist_remove(), clist_find(), clist_find_before(), clist_lpeek(), clist_rpeek() - improved documentation --- core/include/clist.h | 265 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 245 insertions(+), 20 deletions(-) diff --git a/core/include/clist.h b/core/include/clist.h index beda190a5a..b02d06adcf 100644 --- a/core/include/clist.h +++ b/core/include/clist.h @@ -14,13 +14,66 @@ * @file * @brief Circular linked list * - * This file contains a circularly linked list implementation. + * This file contains a circularly and singly linked list implementation. * - * clist_insert(), clist_remove_head() and clist_advance() take constant time. + * Its operations are: * - * Each list is represented as a "clist_node_t". It's only member, the "next" + * operation | runtime | description + * ---------------------|---------|--------------- + * clist_lpush() | O(1) | insert as head (leftmost node) + * clist_lpop() | O(1) | remove and return head (leftmost node) + * clist_rpush() | O(1) | append as tail (rightmost node) + * clist_rpop() | O(n) | remove and return tail (rightmost node) + * clist_find() | O(n) | find and return node + * clist_find_before() | O(n) | find node return node pointing to node + * clist_remove() | O(n) | remove and return node + * + * clist can be used as a traditional list, a queue (FIFO) and a stack (LIFO) using + * fast O(1) operations. + * + * When used as traditional list, in order to traverse, make sure every element + * is only visited once. + * + * Example: + * + * void clist_traverse(clist_node_t *list) { + * clist_node_t *node = list->next; + * if (! node) { + * puts("list empty"); + * return; + * } + * + * do { + * node = node->next; + * // do something with node + * } while (node != list->next); + * } + * + * Or use the clist_foreach() helper function, e.g.,: + * + * static int _print_node(clist_node_t *node) + * { + * printf("0x%08x ", (unsigned)node); + * return 0; + * } + * + * [...] + * clist_foreach(&list, _print_node); + * + * To use clist as a queue, use clist_rpush() for adding elements and clist_lpop() + * for removal. Using clist_lpush() and clist_rpop() is inefficient due to + * clist_rpop()'s O(n) runtime. + * + * To use clist as stack, use clist_lpush()/clist_lpop(). + * + * Implementation details: + * + * Each list is represented as a "clist_node_t". Its only member, the "next" * pointer, points to the last entry in the list, whose "next" pointer points to * the first entry. + * Actual list objects should have a @c clist_node_t as member and then use + * the container_of() macro in list operations. + * See @ref thread_add_to_list() as example. * * @author Kaspar Schleiser */ @@ -28,7 +81,7 @@ #ifndef CLIST_H #define CLIST_H -#include "kernel_defines.h" +#include #include "list.h" #ifdef __cplusplus @@ -40,23 +93,19 @@ * * Used as is as reference to a list. * - * clist stores a pointer to the last element of a list. That way, both - * appending to end of list and removing head can be done in constant time. - * - * Actual list objects should have a @c clist_node_t as member and then use - * the container_of() macro in list operations. - * See @ref thread_add_to_list() as example. */ typedef list_node_t clist_node_t; /** - * @brief inserts *new_node* into *list* + * @brief Appends *new_node* at the end of *list * - * @param[in,out] list Ptr to clist + * @note Complexity: O(1) + * + * @param[in,out] list Pointer to clist * @param[in,out] new_node Node which gets inserted. * Must not be NULL. */ -static inline void clist_insert(clist_node_t *list, clist_node_t *new_node) +static inline void clist_rpush(clist_node_t *list, clist_node_t *new_node) { if (list->next) { new_node->next = list->next->next; @@ -68,13 +117,37 @@ static inline void clist_insert(clist_node_t *list, clist_node_t *new_node) list->next = new_node; } + +/** + * @brief Inserts *new_node* at the beginning of *list + * + * @note Complexity: O(1) + * + * @param[in,out] list Pointer to clist + * @param[in,out] new_node Node which gets inserted. + * Must not be NULL. + */ +static inline void clist_lpush(clist_node_t *list, clist_node_t *new_node) +{ + if (list->next) { + new_node->next = list->next->next; + list->next->next = new_node; + } + else { + new_node->next = new_node; + list->next = new_node; + } +} + /** * @brief Removes and returns first element from list * + * @note Complexity: O(1) + * * @param[in,out] list Pointer to the *list* to remove first element * from. */ -static inline clist_node_t *clist_remove_head(clist_node_t *list) +static inline clist_node_t *clist_lpop(clist_node_t *list) { if (list->next) { clist_node_t *first = list->next->next; @@ -98,23 +171,175 @@ static inline clist_node_t *clist_remove_head(clist_node_t *list) * nodes shifted by one. So second list entry will be * first, first is last. * + * [ A, B, C ] becomes [ B, C, A ] + * + * @note Complexity: O(1) + * * @param[in,out] list The list to work upon. */ -static inline void clist_advance(clist_node_t *list) +static inline void clist_lpoprpush(clist_node_t *list) { if (list->next) { list->next = list->next->next; } } -#if ENABLE_DEBUG /** - * @brief Prints list to stdout. + * @brief Returns first element in list * - * @param[in] clist The list to get printed out. + * @note: Complexity: O(1) + * + * @param[in] list The list to work upon. + * @returns first (leftmost) list element, or NULL if list is empty */ -void clist_print(clist_node_t *clist); -#endif +static inline clist_node_t *clist_lpeek(const clist_node_t *list) +{ + if (list->next) { + return list->next->next; + } +} + +/** + * @brief Returns last element in list + * + * @note: Complexity: O(1) + * + * @param[in] list The list to work upon. + * @returns last (rightmost) list element, or NULL if list is empty + */ +static inline clist_node_t *clist_rpeek(const clist_node_t *list) +{ + return list->next; +} + +/** + * @brief Removes and returns last element from list + * + * @note Complexity: O(n) with n being the number of elements in the list. + * + * @param[in,out] list Pointer to the *list* to remove last element + * from. + */ +static inline clist_node_t *clist_rpop(clist_node_t *list) +{ + if (list->next) { + list_node_t *last = list->next; + while (list->next->next != last) { + clist_advance(list); + } + return clist_lpop(list); + } + else { + return NULL; + } +} + +/** + * @brief Finds node and returns its predecessor + * + * @note Complexity: O(n) + * + * @param[in] list pointer to clist + * @param[in,out] node Node to look for + * Must not be NULL. + * + * @returns predecessor of node if found + * @returns NULL if node is not a list member + */ +static inline clist_node_t *clist_find_before(const clist_node_t *list, const clist_node_t *node) +{ + clist_node_t *pos = list->next; + if (!pos) { + return NULL; + } + do { + pos = pos->next; + if (pos->next == node) { + return pos; + } + } while (pos != list->next); + + return NULL; +} + +/** + * @brief Finds and returns node + * + * @note Complexity: O(n) + * + * @param[in] list pointer to clist + * @param[in,out] node Node to look for + * Must not be NULL. + * + * @returns node if found + * @returns NULL if node is not a list member + */ +static inline clist_node_t *clist_find(const clist_node_t *list, const clist_node_t *node) +{ + clist_node_t *tmp = clist_find_before(list, node); + if (tmp) { + return tmp->next; + } + else { + return NULL; + } +} + +/** + * @brief Finds and removes node + * + * @note Complexity: O(n) + * + * @param[in] list pointer to clist + * @param[in,out] node Node to remove for + * Must not be NULL. + * + * @returns node if found and removed + * @returns NULL if node is not a list member + */ +static inline clist_node_t *clist_remove(clist_node_t *list, clist_node_t *node) +{ + if (list->next) { + if (list->next->next == node) { + return clist_lpop(list); + } + else { + clist_node_t *tmp = clist_find_before(list, node); + if (tmp) { + tmp->next = tmp->next->next; + if (node == list->next) { + list->next = tmp; + } + return node; + } + } + } + + return NULL; +} + +/** + * @brief Traverse clist, call function for each member + * + * If @p func returns non-zero, traversal will be aborted like when calling + * break within a for loop. + * + * @param[in] list List to traverse. + * @param[in] func Function to call for each member. + */ +static inline void clist_foreach(clist_node_t *list, int(*func)(clist_node_t *)) +{ + clist_node_t *node = list->next; + if (! node) { + return; + } + do { + node = node->next; + if (func(node)) { + return; + } + } while (node != list->next); +} #ifdef __cplusplus } From d86c1418422392ca913874d56e6c07fe86b2c0b6 Mon Sep 17 00:00:00 2001 From: Kaspar Schleiser Date: Wed, 13 Jul 2016 00:53:14 +0200 Subject: [PATCH 3/3] core, tests: adapt to changed clist function names --- core/include/clist.h | 2 +- core/sched.c | 4 +- core/thread.c | 2 +- tests/unittests/tests-core/tests-core-clist.c | 205 ++++++++++++++++-- 4 files changed, 192 insertions(+), 21 deletions(-) diff --git a/core/include/clist.h b/core/include/clist.h index b02d06adcf..d2da84b31d 100644 --- a/core/include/clist.h +++ b/core/include/clist.h @@ -225,7 +225,7 @@ static inline clist_node_t *clist_rpop(clist_node_t *list) if (list->next) { list_node_t *last = list->next; while (list->next->next != last) { - clist_advance(list); + clist_lpoprpush(list); } return clist_lpop(list); } diff --git a/core/sched.c b/core/sched.c index 151649aad2..f346152480 100644 --- a/core/sched.c +++ b/core/sched.c @@ -133,7 +133,7 @@ void sched_set_status(thread_t *process, unsigned int status) if (!(process->status >= STATUS_ON_RUNQUEUE)) { DEBUG("sched_set_status: adding thread %" PRIkernel_pid " to runqueue %" PRIu16 ".\n", process->pid, process->priority); - clist_insert(&sched_runqueues[process->priority], &(process->rq_entry)); + clist_rpush(&sched_runqueues[process->priority], &(process->rq_entry)); runqueue_bitcache |= 1 << process->priority; } } @@ -141,7 +141,7 @@ void sched_set_status(thread_t *process, unsigned int status) if (process->status >= STATUS_ON_RUNQUEUE) { DEBUG("sched_set_status: removing thread %" PRIkernel_pid " to runqueue %" PRIu16 ".\n", process->pid, process->priority); - clist_remove_head(&sched_runqueues[process->priority]); + clist_lpop(&sched_runqueues[process->priority]); if (!sched_runqueues[process->priority].next) { runqueue_bitcache &= ~(1 << process->priority); diff --git a/core/thread.c b/core/thread.c index f1ce96b06e..ca50b3e765 100644 --- a/core/thread.c +++ b/core/thread.c @@ -98,7 +98,7 @@ void thread_yield(void) unsigned old_state = irq_disable(); thread_t *me = (thread_t *)sched_active_thread; if (me->status >= STATUS_ON_RUNQUEUE) { - clist_advance(&sched_runqueues[me->priority]); + clist_lpoprpush(&sched_runqueues[me->priority]); } irq_restore(old_state); diff --git a/tests/unittests/tests-core/tests-core-clist.c b/tests/unittests/tests-core/tests-core-clist.c index d9ec7e42fb..21271ef6d2 100644 --- a/tests/unittests/tests-core/tests-core-clist.c +++ b/tests/unittests/tests-core/tests-core-clist.c @@ -22,15 +22,15 @@ static list_node_t test_clist; static void set_up(void) { memset(tests_clist_buf, 0, sizeof(tests_clist_buf)); + test_clist.next = NULL; } -static void test_clist_add_one(void) +static void test_clist_rpush(void) { list_node_t *elem = &(tests_clist_buf[0]); list_node_t *list = &test_clist; - list->next = NULL; - clist_insert(list, elem); + clist_rpush(list, elem); TEST_ASSERT_NOT_NULL(list->next); TEST_ASSERT(list->next->next == list->next); @@ -39,29 +39,130 @@ static void test_clist_add_one(void) static void test_clist_add_two(void) { list_node_t *list = &test_clist; - list->next = NULL; list_node_t *elem = &(tests_clist_buf[1]); - test_clist_add_one(); + test_clist_rpush(); - clist_insert(list, elem); + clist_rpush(list, elem); TEST_ASSERT_NOT_NULL(list->next); TEST_ASSERT(list->next == elem); TEST_ASSERT(list->next->next->next == list->next); } -static void test_clist_remove_head(void) +static void test_clist_add_three(void) +{ + list_node_t *list = &test_clist; + + for (int i = 0; i < 3; i++) { + clist_rpush(list, &(tests_clist_buf[i])); + } + + TEST_ASSERT_NOT_NULL(list->next); + TEST_ASSERT(list->next == &(tests_clist_buf[2])); + TEST_ASSERT(list->next->next == &(tests_clist_buf[0])); + TEST_ASSERT(list->next->next->next == &(tests_clist_buf[1])); + TEST_ASSERT(list->next->next->next->next == &(tests_clist_buf[2])); +} + +static void test_clist_find(void) +{ + list_node_t *list = &test_clist; + + test_clist_add_three(); + + for (int i = 0; i < 3; i++) { + TEST_ASSERT(clist_find(list, &(tests_clist_buf[i])) == &(tests_clist_buf[i])); + } + + TEST_ASSERT_NULL(clist_find(list, &(tests_clist_buf[3]))); +} + +static void test_clist_find_before(void) +{ + list_node_t *list = &test_clist; + + test_clist_add_three(); + + TEST_ASSERT(clist_find_before(list, &(tests_clist_buf[0])) == &(tests_clist_buf[2])); + + for (int i = 1; i < 3; i++) { + TEST_ASSERT(clist_find_before(list, &(tests_clist_buf[i])) == &(tests_clist_buf[i-1])); + } + + TEST_ASSERT_NULL(clist_find_before(list, &(tests_clist_buf[3]))); +} + +static void test_clist_remove(void) +{ + list_node_t *list = &test_clist; + + for (int i = 0; i < 3; i++) { + set_up(); + test_clist_add_three(); + clist_remove(list, &(tests_clist_buf[i])); + + for (int j = 0; j < 3; j++) { + if (i == j) { + TEST_ASSERT_NULL(clist_find(list, &(tests_clist_buf[j]))); + } + else { + TEST_ASSERT(clist_find(list, &(tests_clist_buf[j])) == &(tests_clist_buf[j])); + } + } + } + + /* list now contains 0, 1 */ + TEST_ASSERT(list->next == &(tests_clist_buf[1])); + TEST_ASSERT(list->next->next == &(tests_clist_buf[0])); + + clist_remove(list, &(tests_clist_buf[1])); + TEST_ASSERT(list->next == &(tests_clist_buf[0])); + TEST_ASSERT(list->next->next == &(tests_clist_buf[0])); + + clist_remove(list, &(tests_clist_buf[0])); + TEST_ASSERT_NULL(list->next); +} + +static void test_clist_lpop(void) +{ + list_node_t *list = &test_clist; + + test_clist_add_three(); + + TEST_ASSERT(clist_lpop(list) == &tests_clist_buf[0]); + TEST_ASSERT_NOT_NULL(list->next); + + TEST_ASSERT(clist_lpop(list) == &tests_clist_buf[1]); + TEST_ASSERT_NOT_NULL(list->next); + + TEST_ASSERT(clist_lpop(list) == &tests_clist_buf[2]); + TEST_ASSERT_NULL(list->next); + TEST_ASSERT_NULL(clist_lpop(list)); +} + +static void test_clist_lpush(void) +{ + list_node_t *list = &test_clist; + + test_clist_add_two(); + clist_lpush(list, &tests_clist_buf[2]); + + TEST_ASSERT_NOT_NULL(list->next); + TEST_ASSERT(list->next->next == &tests_clist_buf[2]); +} + +static void test_clist_rpop(void) { list_node_t *list = &test_clist; test_clist_add_two(); - clist_remove_head(list); + clist_rpop(list); TEST_ASSERT_NOT_NULL(list->next); - TEST_ASSERT(list->next->next == &tests_clist_buf[1]); + TEST_ASSERT(list->next->next == &tests_clist_buf[0]); } static void test_clist_remove_two(void) @@ -70,36 +171,106 @@ static void test_clist_remove_two(void) test_clist_add_two(); - clist_remove_head(list); - clist_remove_head(list); + clist_lpop(list); + clist_lpop(list); TEST_ASSERT_NULL(list->next); } -static void test_clist_advance(void) +static void test_clist_lpoprpush(void) { list_node_t *list = &test_clist; list->next = NULL; test_clist_add_two(); - clist_advance(list); + clist_lpoprpush(list); TEST_ASSERT(list->next->next == &tests_clist_buf[1]); - clist_advance(list); + clist_lpoprpush(list); TEST_ASSERT(list->next->next == &tests_clist_buf[0]); } +static int _foreach_called; +static int _foreach_visited[TEST_CLIST_LEN]; +static int _foreach_abort_after = TEST_CLIST_LEN/2; + +static void _foreach_test(clist_node_t *node) +{ + TEST_ASSERT(node == &tests_clist_buf[_foreach_called]); + + for (int i = 0; i < TEST_CLIST_LEN; i++) { + if (node == &tests_clist_buf[i]) { + _foreach_visited[i]++; + break; + } + } + + for (int i = 0; i < TEST_CLIST_LEN; i++) { + if (i <= _foreach_called) { + TEST_ASSERT(_foreach_visited[i] == 1); + } + else { + TEST_ASSERT(_foreach_visited[i] == 0); + } + } + + _foreach_called++; +} + +/* embunit test macros only work within void returning functions, so this + * trampoline function is needed */ +static int _foreach_test_trampoline(clist_node_t *node) +{ + _foreach_test(node); + if (_foreach_called == _foreach_abort_after) { + return 1; + } + else { + return 0; + } +} + +static void test_clist_foreach(void) +{ + list_node_t *list = &test_clist; + + for (int i = 0; i < TEST_CLIST_LEN; i++) { + clist_rpush(list, &tests_clist_buf[i]); + } + + clist_foreach(list, _foreach_test_trampoline); + + TEST_ASSERT(_foreach_called == _foreach_abort_after); + + _foreach_called = 0; + for (int i = 0; i < TEST_CLIST_LEN; i++) { + _foreach_visited[i] = 0; + } + + _foreach_abort_after = (TEST_CLIST_LEN + 1); + clist_foreach(list, _foreach_test_trampoline); + + TEST_ASSERT(_foreach_called == TEST_CLIST_LEN); +} + Test *tests_core_clist_tests(void) { EMB_UNIT_TESTFIXTURES(fixtures) { - new_TestFixture(test_clist_add_one), + new_TestFixture(test_clist_rpush), new_TestFixture(test_clist_add_two), - new_TestFixture(test_clist_remove_head), + new_TestFixture(test_clist_lpop), + new_TestFixture(test_clist_rpop), + new_TestFixture(test_clist_lpush), new_TestFixture(test_clist_remove_two), - new_TestFixture(test_clist_advance), + new_TestFixture(test_clist_add_three), + new_TestFixture(test_clist_find), + new_TestFixture(test_clist_find_before), + new_TestFixture(test_clist_remove), + new_TestFixture(test_clist_lpoprpush), + new_TestFixture(test_clist_foreach), }; EMB_UNIT_TESTCALLER(core_clist_tests, set_up, NULL,