Skip to content

Commit

Permalink
Makes bs_dllist_for_each handle in-callback node deletion, and fixes …
Browse files Browse the repository at this point in the history
…a ptr_vector out-of-bound read. (#22)

* Makes bs_dllist_for_each handle per-iteration node destroy.

* Fixes a off-by-one leading to accessing out-of-bounds memory.
  • Loading branch information
phkaeser authored Jun 29, 2024
1 parent 179fc58 commit 4e80eee
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
41 changes: 36 additions & 5 deletions dllist.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,11 @@ void bs_dllist_for_each(
void (*func)(bs_dllist_node_t *dlnode_ptr, void *ud_ptr),
void *ud_ptr)
{
for (bs_dllist_node_t *dlnode_ptr = list_ptr->head_ptr;
dlnode_ptr != NULL;
dlnode_ptr = dlnode_ptr->next_ptr) {
func(dlnode_ptr, ud_ptr);
bs_dllist_node_t *dlnode_ptr = list_ptr->head_ptr;
while (NULL != dlnode_ptr) {
bs_dllist_node_t *current_dlnode_ptr = dlnode_ptr;
dlnode_ptr = dlnode_ptr->next_ptr;
func(current_dlnode_ptr, ud_ptr);
}
}

Expand Down Expand Up @@ -285,6 +286,7 @@ static void bs_dllist_test_remove(bs_test_t *test_ptr);
static void bs_dllist_test_insert(bs_test_t *test_ptr);
static void bs_dllist_test_find(bs_test_t *test_ptr);
static void bs_dllist_test_for_each(bs_test_t *test_ptr);
static void bs_dllist_test_for_each_dtor(bs_test_t *test_ptr);

const bs_test_case_t bs_dllist_test_cases[] = {
{ 1, "push/pop back", bs_dllist_test_back },
Expand All @@ -293,6 +295,7 @@ const bs_test_case_t bs_dllist_test_cases[] = {
{ 1, "insert", bs_dllist_test_insert },
{ 1, "find", bs_dllist_test_find },
{ 1, "for_each", bs_dllist_test_for_each },
{ 1, "for_each_dtor", bs_dllist_test_for_each_dtor },
{ 0, NULL, NULL }
};

Expand Down Expand Up @@ -470,7 +473,9 @@ static bool test_find(bs_dllist_node_t *dlnode_ptr, void *ud_ptr)

/* ------------------------------------------------------------------------- */
/** Function to use in the test for @ref bs_dllist_for_each. */
static void test_for_each(__UNUSED__ bs_dllist_node_t *dlnode_ptr, void *ud_ptr)
static void test_for_each(
__UNUSED__ bs_dllist_node_t *dlnode_ptr,
void *ud_ptr)
{
*((int*)ud_ptr) += 1;
}
Expand Down Expand Up @@ -526,4 +531,30 @@ void bs_dllist_test_for_each(bs_test_t *test_ptr)
BS_TEST_VERIFY_EQ(test_ptr, 2, outcome);
}

/* ------------------------------------------------------------------------- */
/** Helper: dtor for the allocated @ref bs_dllist_node_t. */
static void test_for_each_dtor(bs_dllist_node_t *dlnode_ptr,
__UNUSED__ void *ud_ptr)
{
bs_dllist_remove(ud_ptr, dlnode_ptr);
free(dlnode_ptr);
}

/** Runs bs_dllist_for_each with a dtor, ensuring no invalid access. */
void bs_dllist_test_for_each_dtor(bs_test_t *test_ptr)
{
bs_dllist_t list = {};

for (int i = 0; i < 5; ++i) {
bs_dllist_node_t *node_ptr = BS_ASSERT_NOTNULL(
calloc(1, sizeof(bs_dllist_node_t)));
bs_dllist_push_back(&list, node_ptr);
}
BS_TEST_VERIFY_EQ(test_ptr, 5, bs_dllist_size(&list));

bs_dllist_for_each(&list, test_for_each_dtor, &list);
BS_TEST_VERIFY_TRUE(test_ptr, bs_dllist_empty(&list));
}


/* == End of dllist.c ===================================================== */
3 changes: 3 additions & 0 deletions dllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ bs_dllist_node_t *bs_dllist_find(
/**
* Runs |func()| for each of the nodes in |list_ptr|.
*
* The list iterator is kept safe, so it is permitted to remove the called-back
* node from the list: Useful for eg. destroying all list elements.
*
* @param list_ptr
* @param func
* @param ud_ptr
Expand Down
9 changes: 8 additions & 1 deletion ptr_vector.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ bool bs_ptr_vector_erase(bs_ptr_vector_t *ptr_vector_ptr,
if (pos + 1 < ptr_vector_ptr->consumed) {
memmove(ptr_vector_ptr->elements_ptr + pos,
ptr_vector_ptr->elements_ptr + pos + 1,
(ptr_vector_ptr->consumed - pos) * sizeof(void*));
(ptr_vector_ptr->consumed - pos - 1) * sizeof(void*));
}
--ptr_vector_ptr->consumed;
return true;
Expand Down Expand Up @@ -172,7 +172,14 @@ void large_test(bs_test_t *test_ptr)
}

for (size_t i = 0; i < 2 * INITIAL_SIZE; ++i) {
BS_TEST_VERIFY_EQ(test_ptr, &e[i], bs_ptr_vector_at(&vec, 0));
BS_TEST_VERIFY_TRUE(test_ptr, bs_ptr_vector_erase(&vec, 0));
if (0 < bs_ptr_vector_size(&vec)) {
BS_TEST_VERIFY_EQ(
test_ptr,
&e[2 * INITIAL_SIZE - 1],
bs_ptr_vector_at(&vec, bs_ptr_vector_size(&vec) - 1));
}
}

bs_ptr_vector_fini(&vec);
Expand Down

0 comments on commit 4e80eee

Please sign in to comment.