From: Daniel Stenberg Date: Thu, 15 Aug 2024 14:13:23 +0000 (+0200) Subject: splay: use access functions, add asserts, use Curl_timediff X-Git-Tag: curl-8_10_0~239 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dcb51bafabf9d0776135de9111c1a3fe437d570c;p=thirdparty%2Fcurl.git splay: use access functions, add asserts, use Curl_timediff - add set/get functions for the custom data in a tree node - use Curl_timediff for time comparisons instead of the custom macro, as they do the exact same things. - add asserts to catch mistakes better - updated test 1309 accordingly Closes #14562 --- diff --git a/lib/multi.c b/lib/multi.c index b07ebe5d07..f286325c23 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -2665,7 +2665,7 @@ statemachine_end: CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles) { CURLMcode returncode = CURLM_OK; - struct Curl_tree *t; + struct Curl_tree *t = NULL; struct curltime now = Curl_now(); struct Curl_llist_node *e; struct Curl_llist_node *n = NULL; @@ -2716,7 +2716,7 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles) multi->timetree = Curl_splaygetbest(now, multi->timetree, &t); if(t) { /* the removed may have another timeout in queue */ - struct Curl_easy *data = t->payload; + struct Curl_easy *data = Curl_splayget(t); if(data->mstate == MSTATE_PENDING) { bool stream_unused; CURLcode result_unused; @@ -2726,7 +2726,7 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles) move_pending_to_connect(multi, data); } } - (void)add_next_timeout(now, multi, t->payload); + (void)add_next_timeout(now, multi, Curl_splayget(t)); } } while(t); @@ -3153,7 +3153,7 @@ static CURLMcode multi_socket(struct Curl_multi *multi, { CURLMcode result = CURLM_OK; struct Curl_easy *data = NULL; - struct Curl_tree *t; + struct Curl_tree *t = NULL; struct curltime now = Curl_now(); bool run_conn_cache = FALSE; SIGPIPE_VARIABLE(pipe_st); @@ -3256,8 +3256,8 @@ static CURLMcode multi_socket(struct Curl_multi *multi, multi->timetree = Curl_splaygetbest(now, multi->timetree, &t); if(t) { - data = t->payload; /* assign this for next loop */ - (void)add_next_timeout(now, multi, t->payload); + data = Curl_splayget(t); /* assign this for next loop */ + (void)add_next_timeout(now, multi, data); } } while(t); @@ -3408,7 +3408,10 @@ static CURLMcode multi_timeout(struct Curl_multi *multi, /* splay the lowest to the bottom */ multi->timetree = Curl_splay(tv_zero, multi->timetree); - if(Curl_splaycomparekeys(multi->timetree->key, now) > 0) { + /* 'multi->timetree' will be non-NULL here but the compilers sometimes + yell at us if we assume so */ + if(multi->timetree && + Curl_timediff_us(multi->timetree->key, now) > 0) { /* some time left before expiration */ timediff_t diff = Curl_timediff_ceil(multi->timetree->key, now); /* this should be safe even on 32-bit archs, as we do not use that @@ -3454,7 +3457,7 @@ CURLMcode Curl_update_timer(struct Curl_multi *multi) } if(timeout_ms < 0) { static const struct curltime none = {0, 0}; - if(Curl_splaycomparekeys(none, multi->timer_lastcall)) { + if(Curl_timediff_us(none, multi->timer_lastcall)) { multi->timer_lastcall = none; /* there is no timeout now but there was one previously, tell the app to disable it */ @@ -3474,7 +3477,7 @@ CURLMcode Curl_update_timer(struct Curl_multi *multi) * timeout we got the (relative) time-out time for. We can thus easily check * if this is the same (fixed) time as we got in a previous call and then * avoid calling the callback again. */ - if(Curl_splaycomparekeys(multi->timetree->key, multi->timer_lastcall) == 0) + if(Curl_timediff_us(multi->timetree->key, multi->timer_lastcall) == 0) return CURLM_OK; multi->timer_lastcall = multi->timetree->key; @@ -3616,7 +3619,7 @@ void Curl_expire(struct Curl_easy *data, timediff_t milli, expire_id id) /* Indicate that we are in the splay tree and insert the new timer expiry value since it is our local minimum. */ *nowp = set; - data->state.timenode.payload = data; + Curl_splayset(&data->state.timenode, data); multi->timetree = Curl_splayinsert(*nowp, multi->timetree, &data->state.timenode); } diff --git a/lib/splay.c b/lib/splay.c index 99bb149971..5e27b08a6c 100644 --- a/lib/splay.c +++ b/lib/splay.c @@ -24,6 +24,7 @@ #include "curl_setup.h" +#include "timeval.h" #include "splay.h" /* @@ -33,7 +34,7 @@ * zero : when i is equal to j * positive when : when i is larger than j */ -#define compare(i,j) Curl_splaycomparekeys((i),(j)) +#define compare(i,j) Curl_timediff_us(i,j) /* * Splay using the key i (which may or may not be in the tree.) The starting @@ -45,12 +46,12 @@ struct Curl_tree *Curl_splay(struct curltime i, struct Curl_tree N, *l, *r, *y; if(!t) - return t; + return NULL; N.smaller = N.larger = NULL; l = r = &N; for(;;) { - long comp = compare(i, t->key); + timediff_t comp = compare(i, t->key); if(comp < 0) { if(!t->smaller) break; @@ -106,11 +107,11 @@ struct Curl_tree *Curl_splayinsert(struct curltime i, ~0, -1 }; /* will *NEVER* appear */ - if(!node) - return t; + DEBUGASSERT(node); if(t) { t = Curl_splay(i, t); + DEBUGASSERT(t); if(compare(i, t->key) == 0) { /* There already exists a node in the tree with the very same key. Build a doubly-linked circular list of nodes. We add the new 'node' struct @@ -166,6 +167,7 @@ struct Curl_tree *Curl_splaygetbest(struct curltime i, /* find smallest */ t = Curl_splay(tv_zero, t); + DEBUGASSERT(t); if(compare(i, t->key) < 0) { /* even the smallest is too big */ *removed = NULL; @@ -217,9 +219,11 @@ int Curl_splayremove(struct Curl_tree *t, }; /* will *NEVER* appear */ struct Curl_tree *x; - if(!t || !removenode) + if(!t) return 1; + DEBUGASSERT(removenode); + if(compare(KEY_NOTUSED, removenode->key) == 0) { /* Key set to NOTUSED means it is a subnode within a 'same' linked list and thus we can unlink it easily. */ @@ -238,6 +242,7 @@ int Curl_splayremove(struct Curl_tree *t, } t = Curl_splay(removenode->key, t); + DEBUGASSERT(t); /* First make sure that we got the same root node as the one we want to remove, as otherwise we might be trying to remove a node that @@ -268,6 +273,7 @@ int Curl_splayremove(struct Curl_tree *t, x = t->larger; else { x = Curl_splay(removenode->key, t->smaller); + DEBUGASSERT(x); x->larger = t->larger; } } @@ -276,3 +282,16 @@ int Curl_splayremove(struct Curl_tree *t, return 0; } + +/* set and get the custom payload for this tree node */ +void Curl_splayset(struct Curl_tree *node, void *payload) +{ + DEBUGASSERT(node); + node->ptr = payload; +} + +void *Curl_splayget(struct Curl_tree *node) +{ + DEBUGASSERT(node); + return node->ptr; +} diff --git a/lib/splay.h b/lib/splay.h index 20b2bb69a9..b8c9360e57 100644 --- a/lib/splay.h +++ b/lib/splay.h @@ -26,13 +26,14 @@ #include "curl_setup.h" #include "timeval.h" +/* only use function calls to access this struct */ struct Curl_tree { struct Curl_tree *smaller; /* smaller node */ struct Curl_tree *larger; /* larger node */ struct Curl_tree *samen; /* points to the next node with identical key */ struct Curl_tree *samep; /* points to the prev node with identical key */ - struct curltime key; /* this node's "sort" key */ - void *payload; /* data the splay code does not care about */ + struct curltime key; /* this node's "sort" key */ + void *ptr; /* data the splay code does not care about */ }; struct Curl_tree *Curl_splay(struct curltime i, @@ -50,9 +51,8 @@ int Curl_splayremove(struct Curl_tree *t, struct Curl_tree *removenode, struct Curl_tree **newroot); -#define Curl_splaycomparekeys(i,j) ( ((i.tv_sec) < (j.tv_sec)) ? -1 : \ - ( ((i.tv_sec) > (j.tv_sec)) ? 1 : \ - ( ((i.tv_usec) < (j.tv_usec)) ? -1 : \ - ( ((i.tv_usec) > (j.tv_usec)) ? 1 : 0)))) +/* set and get the custom payload for this tree node */ +void Curl_splayset(struct Curl_tree *node, void *payload); +void *Curl_splayget(struct Curl_tree *node); #endif /* HEADER_CURL_SPLAY_H */ diff --git a/tests/unit/unit1309.c b/tests/unit/unit1309.c index 5c5801abd6..0ae71205be 100644 --- a/tests/unit/unit1309.c +++ b/tests/unit/unit1309.c @@ -88,7 +88,7 @@ UNITTEST_START key.tv_sec = 0; key.tv_usec = (541*i)%1023; storage[i] = key.tv_usec; - nodes[i].payload = &storage[i]; + Curl_splayset(&nodes[i], &storage[i]); root = Curl_splayinsert(key, root, &nodes[i]); } @@ -100,7 +100,7 @@ UNITTEST_START printf("Tree look:\n"); splayprint(root, 0, 1); printf("remove pointer %d, payload %zu\n", rem, - *(size_t *)nodes[rem].payload); + *(size_t *)Curl_splayget(&nodes[rem])); rc = Curl_splayremove(root, &nodes[rem], &root); if(rc) { /* failed! */ @@ -121,7 +121,7 @@ UNITTEST_START /* add some nodes with the same key */ for(j = 0; j <= i % 3; j++) { storage[i * 3 + j] = key.tv_usec*10 + j; - nodes[i * 3 + j].payload = &storage[i * 3 + j]; + Curl_splayset(&nodes[i * 3 + j], &storage[i * 3 + j]); root = Curl_splayinsert(key, root, &nodes[i * 3 + j]); } } @@ -133,8 +133,8 @@ UNITTEST_START root = Curl_splaygetbest(tv_now, root, &removed); while(removed) { printf("removed payload %zu[%zu]\n", - (*(size_t *)removed->payload) / 10, - (*(size_t *)removed->payload) % 10); + *(size_t *)Curl_splayget(removed) / 10, + *(size_t *)Curl_splayget(removed) % 10); root = Curl_splaygetbest(tv_now, root, &removed); } }