llist: remove direct struct accesses, use only functions

- Turned them all into functions to also do asserts etc.

- The llist related structs got all their fields renamed in order to make
  sure no existing code remains using direct access.

- Each list node struct now points back to the list it "lives in", so
  Curl_node_remove() no longer needs the list pointer.

- Rename the node struct and some of the access functions.

- Added lots of ASSERTs to verify API being used correctly

- Fix some cases of API misuse

Add docs/LLIST.md documenting the internal linked list API.

Closes #14485
This commit is contained in:
Daniel Stenberg 2024-08-10 23:27:25 +02:00
parent 6f00a05e89
commit ba235ab269
No known key found for this signature in database
GPG key ID: 5CC908FDB71E12C2
27 changed files with 725 additions and 428 deletions

View file

@ -52,14 +52,14 @@ UNITTEST_START
int unusedData_case1 = 1;
int unusedData_case2 = 2;
int unusedData_case3 = 3;
struct Curl_llist_element case1_list;
struct Curl_llist_element case2_list;
struct Curl_llist_element case3_list;
struct Curl_llist_element case4_list;
struct Curl_llist_element *head;
struct Curl_llist_element *element_next;
struct Curl_llist_element *element_prev;
struct Curl_llist_element *to_remove;
struct Curl_llist_node case1_list;
struct Curl_llist_node case2_list;
struct Curl_llist_node case3_list;
struct Curl_llist_node case4_list;
struct Curl_llist_node *head;
struct Curl_llist_node *element_next;
struct Curl_llist_node *element_prev;
struct Curl_llist_node *to_remove;
size_t llist_size = Curl_llist_count(&llist);
/**
@ -73,11 +73,9 @@ UNITTEST_START
* 4: list dtor will be NULL
*/
fail_unless(llist.size == 0, "list initial size should be zero");
fail_unless(llist.head == NULL, "list head should initiate to NULL");
fail_unless(llist.tail == NULL, "list tail should initiate to NULL");
fail_unless(llist.dtor == test_Curl_llist_dtor,
"list dtor should initiate to test_Curl_llist_dtor");
fail_unless(Curl_llist_count(&llist) == 0, "list initial size should be zero");
fail_unless(Curl_llist_head(&llist) == NULL, "list head should initiate to NULL");
fail_unless(Curl_llist_tail(&llist) == NULL, "list tail should initiate to NULL");
/**
* testing Curl_llist_insert_next
@ -89,15 +87,15 @@ UNITTEST_START
* 3: list tail will be the same as list head
*/
Curl_llist_insert_next(&llist, llist.head, &unusedData_case1, &case1_list);
Curl_llist_insert_next(&llist, Curl_llist_head(&llist), &unusedData_case1, &case1_list);
fail_unless(Curl_llist_count(&llist) == 1,
"List size should be 1 after adding a new element");
/* test that the list head data holds my unusedData */
fail_unless(llist.head->ptr == &unusedData_case1,
fail_unless(Curl_node_elem(Curl_llist_head(&llist)) == &unusedData_case1,
"head ptr should be first entry");
/* same goes for the list tail */
fail_unless(llist.tail == llist.head,
fail_unless(Curl_llist_tail(&llist) == Curl_llist_head(&llist),
"tail and head should be the same");
/**
@ -109,11 +107,12 @@ UNITTEST_START
* 2: the list tail should be our newly created element
*/
Curl_llist_insert_next(&llist, llist.head,
Curl_llist_insert_next(&llist, Curl_llist_head(&llist),
&unusedData_case3, &case3_list);
fail_unless(llist.head->next->ptr == &unusedData_case3,
fail_unless(Curl_node_elem(Curl_node_next(Curl_llist_head(&llist))) ==
&unusedData_case3,
"the node next to head is not getting set correctly");
fail_unless(llist.tail->ptr == &unusedData_case3,
fail_unless(Curl_node_elem(Curl_llist_tail(&llist)) == &unusedData_case3,
"the list tail is not getting set correctly");
/**
@ -125,15 +124,16 @@ UNITTEST_START
* 2: the list tail should different from newly created element
*/
Curl_llist_insert_next(&llist, llist.head,
Curl_llist_insert_next(&llist, Curl_llist_head(&llist),
&unusedData_case2, &case2_list);
fail_unless(llist.head->next->ptr == &unusedData_case2,
fail_unless(Curl_node_elem(Curl_node_next(Curl_llist_head(&llist))) ==
&unusedData_case2,
"the node next to head is not getting set correctly");
/* better safe than sorry, check that the tail isn't corrupted */
fail_unless(llist.tail->ptr != &unusedData_case2,
fail_unless(Curl_node_elem(Curl_llist_tail(&llist)) != &unusedData_case2,
"the list tail is not getting set correctly");
/* unit tests for Curl_llist_remove */
/* unit tests for Curl_node_remove */
/**
* case 1:
@ -144,19 +144,19 @@ UNITTEST_START
* 3: "new" head's previous will be NULL
*/
head = llist.head;
head = Curl_llist_head(&llist);
abort_unless(head, "llist.head is NULL");
element_next = head->next;
element_next = Curl_node_next(head);
llist_size = Curl_llist_count(&llist);
Curl_llist_remove(&llist, llist.head, NULL);
Curl_node_remove(Curl_llist_head(&llist));
fail_unless(Curl_llist_count(&llist) == (llist_size-1),
"llist size not decremented as expected");
fail_unless(llist.head == element_next,
"llist new head not modified properly");
abort_unless(llist.head, "llist.head is NULL");
fail_unless(llist.head->prev == NULL,
"llist size not decremented as expected");
fail_unless(Curl_llist_head(&llist) == element_next,
"llist new head not modified properly");
abort_unless(Curl_llist_head(&llist), "llist.head is NULL");
fail_unless(Curl_node_prev(Curl_llist_head(&llist)) == NULL,
"new head previous not set to null");
/**
@ -169,20 +169,20 @@ UNITTEST_START
* 2: element->previous->next will be element->next
* 3: element->next->previous will be element->previous
*/
Curl_llist_insert_next(&llist, llist.head, &unusedData_case3,
Curl_llist_insert_next(&llist, Curl_llist_head(&llist), &unusedData_case3,
&case4_list);
llist_size = Curl_llist_count(&llist);
fail_unless(llist_size == 3, "should be 3 list members");
to_remove = llist.head->next;
to_remove = Curl_node_next(Curl_llist_head(&llist));
abort_unless(to_remove, "to_remove is NULL");
element_next = to_remove->next;
element_prev = to_remove->prev;
Curl_llist_remove(&llist, to_remove, NULL);
fail_unless(element_prev->next == element_next,
element_next = Curl_node_next(to_remove);
element_prev = Curl_node_prev(to_remove);
Curl_node_uremove(to_remove, NULL);
fail_unless(Curl_node_next(element_prev) == element_next,
"element previous->next is not being adjusted");
abort_unless(element_next, "element_next is NULL");
fail_unless(element_next->prev == element_prev,
fail_unless(Curl_node_prev(element_next) == element_prev,
"element next->previous is not being adjusted");
/**
@ -195,10 +195,10 @@ UNITTEST_START
* 4: list->tail will be tail->previous
*/
to_remove = llist.tail;
element_prev = to_remove->prev;
Curl_llist_remove(&llist, to_remove, NULL);
fail_unless(llist.tail == element_prev,
to_remove = Curl_llist_tail(&llist);
element_prev = Curl_node_prev(to_remove);
Curl_node_remove(to_remove);
fail_unless(Curl_llist_tail(&llist) == element_prev,
"llist tail is not being adjusted when removing tail");
/**
@ -210,11 +210,11 @@ UNITTEST_START
* 3: list tail will be null
*/
to_remove = llist.head;
Curl_llist_remove(&llist, to_remove, NULL);
fail_unless(llist.head == NULL,
to_remove = Curl_llist_head(&llist);
Curl_node_remove(to_remove);
fail_unless(Curl_llist_head(&llist) == NULL,
"llist head is not NULL while the llist is empty");
fail_unless(llist.tail == NULL,
fail_unless(Curl_llist_tail(&llist) == NULL,
"llist tail is not NULL while the llist is empty");
/**
@ -229,10 +229,10 @@ UNITTEST_START
fail_unless(Curl_llist_count(&llist) == 1,
"List size should be 1 after appending a new element");
/* test that the list head data holds my unusedData */
fail_unless(llist.head->ptr == &unusedData_case1,
fail_unless(Curl_node_elem(Curl_llist_head(&llist)) == &unusedData_case1,
"head ptr should be first entry");
/* same goes for the list tail */
fail_unless(llist.tail == llist.head,
fail_unless(Curl_llist_tail(&llist) == Curl_llist_head(&llist),
"tail and head should be the same");
/**
@ -244,9 +244,10 @@ UNITTEST_START
* 2: the list tail should be the newly created element
*/
Curl_llist_append(&llist, &unusedData_case2, &case2_list);
fail_unless(llist.head->next->ptr == &unusedData_case2,
fail_unless(Curl_node_elem(Curl_node_next(Curl_llist_head(&llist))) ==
&unusedData_case2,
"the node next to head is not getting set correctly");
fail_unless(llist.tail->ptr == &unusedData_case2,
fail_unless(Curl_node_elem(Curl_llist_tail(&llist)) == &unusedData_case2,
"the list tail is not getting set correctly");
/**
@ -258,13 +259,12 @@ UNITTEST_START
* 2: the list tail should be the newly created element
*/
Curl_llist_append(&llist, &unusedData_case3, &case3_list);
fail_unless(llist.head->next->ptr == &unusedData_case2,
fail_unless(Curl_node_elem(Curl_node_next(Curl_llist_head(&llist))) ==
&unusedData_case2,
"the node next to head did not stay the same");
fail_unless(llist.tail->ptr == &unusedData_case3,
fail_unless(Curl_node_elem(Curl_llist_tail(&llist)) == &unusedData_case3,
"the list tail is not getting set correctly");
Curl_llist_destroy(&llist, NULL);
Curl_llist_destroy(&llist_destination, NULL);
}

View file

@ -57,51 +57,51 @@ UNITTEST_START
fail_if(!curl, "curl_easy_init");
goto fail;
}
fail_unless(asi->list.size == 4, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 4, "wrong number of entries");
msnprintf(outname, sizeof(outname), "%s-out", arg);
result = Curl_altsvc_parse(curl, asi, "h2=\"example.com:8080\"\r\n",
ALPN_h1, "example.org", 8080);
fail_if(result, "Curl_altsvc_parse() failed!");
fail_unless(asi->list.size == 5, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 5, "wrong number of entries");
result = Curl_altsvc_parse(curl, asi, "h3=\":8080\"\r\n",
ALPN_h1, "2.example.org", 8080);
fail_if(result, "Curl_altsvc_parse(2) failed!");
fail_unless(asi->list.size == 6, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 6, "wrong number of entries");
result = Curl_altsvc_parse(curl, asi,
"h2=\"example.com:8080\", h3=\"yesyes.com\"\r\n",
ALPN_h1, "3.example.org", 8080);
fail_if(result, "Curl_altsvc_parse(3) failed!");
/* that one should make two entries */
fail_unless(asi->list.size == 8, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 8, "wrong number of entries");
result = Curl_altsvc_parse(curl, asi,
"h2=\"example.com:443\"; ma = 120;\r\n",
ALPN_h2, "example.org", 80);
fail_if(result, "Curl_altsvc_parse(4) failed!");
fail_unless(asi->list.size == 9, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 9, "wrong number of entries");
/* quoted 'ma' value */
result = Curl_altsvc_parse(curl, asi,
"h2=\"example.net:443\"; ma=\"180\";\r\n",
ALPN_h2, "example.net", 80);
fail_if(result, "Curl_altsvc_parse(4) failed!");
fail_unless(asi->list.size == 10, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 10, "wrong number of entries");
result =
Curl_altsvc_parse(curl, asi,
"h2=\":443\", h3=\":443\"; ma = 120; persist = 1\r\n",
ALPN_h1, "curl.se", 80);
fail_if(result, "Curl_altsvc_parse(5) failed!");
fail_unless(asi->list.size == 12, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 12, "wrong number of entries");
/* clear that one again and decrease the counter */
result = Curl_altsvc_parse(curl, asi, "clear;\r\n",
ALPN_h1, "curl.se", 80);
fail_if(result, "Curl_altsvc_parse(6) failed!");
fail_unless(asi->list.size == 10, "wrong number of entries");
fail_unless(Curl_llist_count(&asi->list) == 10, "wrong number of entries");
Curl_altsvc_save(curl, asi, outname);

View file

@ -118,6 +118,7 @@ static void showsts(struct stsentry *e, const char *chost)
}
UNITTEST_START
{
CURLcode result;
struct stsentry *e;
struct hsts *h = Curl_hsts_init();
@ -159,7 +160,7 @@ UNITTEST_START
showsts(e, chost);
}
printf("Number of entries: %zu\n", h->list.size);
printf("Number of entries: %zu\n", Curl_llist_count(&h->list));
/* verify that it is exists for 7 seconds */
chost = "expire.example";
@ -174,6 +175,6 @@ UNITTEST_START
Curl_hsts_cleanup(&h);
curl_easy_cleanup(easy);
curl_global_cleanup();
}
UNITTEST_STOP
#endif