From 254f7bd78a1ab248329def2cbbbc983e7f6d2495 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 10 Dec 2021 12:54:17 +0100 Subject: hash: lazy-alloc the table in Curl_hash_add() This makes Curl_hash_init() infallible which saves error paths. Closes #8132 --- lib/conncache.c | 13 +++------ lib/hash.c | 75 ++++++++++++++++++++++++++++++--------------------- lib/hash.h | 12 ++++----- lib/hostip.c | 8 +++--- lib/hostip.h | 4 +-- lib/multi.c | 19 +++++-------- lib/share.c | 6 +---- tests/unit/unit1305.c | 8 +----- tests/unit/unit1602.c | 7 ++--- tests/unit/unit1603.c | 7 ++--- 10 files changed, 77 insertions(+), 82 deletions(-) diff --git a/lib/conncache.c b/lib/conncache.c index f5ba8ff70..fec1937f0 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -113,21 +113,16 @@ static void free_bundle_hash_entry(void *freethis) int Curl_conncache_init(struct conncache *connc, int size) { - int rc; - /* allocate a new easy handle to use when closing cached connections */ connc->closure_handle = curl_easy_init(); if(!connc->closure_handle) return 1; /* bad */ - rc = Curl_hash_init(&connc->hash, size, Curl_hash_str, - Curl_str_key_compare, free_bundle_hash_entry); - if(rc) - Curl_close(&connc->closure_handle); - else - connc->closure_handle->state.conn_cache = connc; + Curl_hash_init(&connc->hash, size, Curl_hash_str, + Curl_str_key_compare, free_bundle_hash_entry); + connc->closure_handle->state.conn_cache = connc; - return rc; + return 0; /* good */ } void Curl_conncache_destroy(struct conncache *connc) diff --git a/lib/hash.c b/lib/hash.c index f04c46284..884890694 100644 --- a/lib/hash.c +++ b/lib/hash.c @@ -53,32 +53,25 @@ hash_element_dtor(void *user, void *element) * @unittest: 1602 * @unittest: 1603 */ -int +void Curl_hash_init(struct Curl_hash *h, int slots, hash_function hfunc, comp_function comparator, Curl_hash_dtor dtor) { - if(!slots || !hfunc || !comparator ||!dtor) { - return 1; /* failure */ - } + DEBUGASSERT(h); + DEBUGASSERT(slots); + DEBUGASSERT(hfunc); + DEBUGASSERT(comparator); + DEBUGASSERT(dtor); + h->table = NULL; h->hash_func = hfunc; h->comp_func = comparator; h->dtor = dtor; h->size = 0; h->slots = slots; - - h->table = malloc(slots * sizeof(struct Curl_llist)); - if(h->table) { - int i; - for(i = 0; i < slots; ++i) - Curl_llist_init(&h->table[i], (Curl_llist_dtor) hash_element_dtor); - return 0; /* fine */ - } - h->slots = 0; - return 1; /* failure */ } static struct Curl_hash_element * @@ -98,8 +91,9 @@ mk_hash_element(const void *key, size_t key_len, const void *p) #define FETCH_LIST(x,y,z) &x->table[x->hash_func(y, z, x->slots)] -/* Insert the data in the hash. If there already was a match in the hash, - * that data is replaced. +/* Insert the data in the hash. If there already was a match in the hash, that + * data is replaced. This function also "lazily" allocates the table if + * needed, as it isn't done in the _init function (anymore). * * @unittest: 1305 * @unittest: 1602 @@ -111,7 +105,18 @@ Curl_hash_add(struct Curl_hash *h, void *key, size_t key_len, void *p) struct Curl_hash_element *he; struct Curl_llist_element *le; struct Curl_llist *l; + + DEBUGASSERT(h); DEBUGASSERT(h->slots); + if(!h->table) { + int i; + h->table = malloc(h->slots * sizeof(struct Curl_llist)); + if(!h->table) + return NULL; /* OOM */ + for(i = 0; i < h->slots; ++i) + Curl_llist_init(&h->table[i], hash_element_dtor); + } + l = FETCH_LIST(h, key, key_len); for(le = l->head; le; le = le->next) { @@ -142,15 +147,19 @@ int Curl_hash_delete(struct Curl_hash *h, void *key, size_t key_len) { struct Curl_llist_element *le; struct Curl_llist *l; + + DEBUGASSERT(h); DEBUGASSERT(h->slots); - l = FETCH_LIST(h, key, key_len); + if(h->table) { + l = FETCH_LIST(h, key, key_len); - for(le = l->head; le; le = le->next) { - struct Curl_hash_element *he = le->ptr; - if(h->comp_func(he->key, he->key_len, key, key_len)) { - Curl_llist_remove(l, le, (void *) h); - --h->size; - return 0; + for(le = l->head; le; le = le->next) { + struct Curl_hash_element *he = le->ptr; + if(h->comp_func(he->key, he->key_len, key, key_len)) { + Curl_llist_remove(l, le, (void *) h); + --h->size; + return 0; + } } } return 1; @@ -166,7 +175,8 @@ Curl_hash_pick(struct Curl_hash *h, void *key, size_t key_len) struct Curl_llist_element *le; struct Curl_llist *l; - if(h) { + DEBUGASSERT(h); + if(h->table) { DEBUGASSERT(h->slots); l = FETCH_LIST(h, key, key_len); for(le = l->head; le; le = le->next) { @@ -209,13 +219,13 @@ Curl_hash_apply(Curl_hash *h, void *user, void Curl_hash_destroy(struct Curl_hash *h) { - int i; - - for(i = 0; i < h->slots; ++i) { - Curl_llist_destroy(&h->table[i], (void *) h); + if(h->table) { + int i; + for(i = 0; i < h->slots; ++i) { + Curl_llist_destroy(&h->table[i], (void *) h); + } + Curl_safefree(h->table); } - - Curl_safefree(h->table); h->size = 0; h->slots = 0; } @@ -240,7 +250,7 @@ Curl_hash_clean_with_criterium(struct Curl_hash *h, void *user, struct Curl_llist *list; int i; - if(!h) + if(!h || !h->table) return; for(i = 0; i < h->slots; ++i) { @@ -295,6 +305,9 @@ Curl_hash_next_element(struct Curl_hash_iterator *iter) { struct Curl_hash *h = iter->hash; + if(!h->table) + return NULL; /* empty hash, nothing to return */ + /* Get the next element in the current list, if any */ if(iter->current_element) iter->current_element = iter->current_element->next; diff --git a/lib/hash.h b/lib/hash.h index b7f828e07..e166916a9 100644 --- a/lib/hash.h +++ b/lib/hash.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2021, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -69,11 +69,11 @@ struct Curl_hash_iterator { struct Curl_llist_element *current_element; }; -int Curl_hash_init(struct Curl_hash *h, - int slots, - hash_function hfunc, - comp_function comparator, - Curl_hash_dtor dtor); +void Curl_hash_init(struct Curl_hash *h, + int slots, + hash_function hfunc, + comp_function comparator, + Curl_hash_dtor dtor); void *Curl_hash_add(struct Curl_hash *h, void *key, size_t key_len, void *p); int Curl_hash_delete(struct Curl_hash *h, void *key, size_t key_len); diff --git a/lib/hostip.c b/lib/hostip.c index c33c9af9d..5d942dca0 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -977,12 +977,12 @@ static void freednsentry(void *freethis) } /* - * Curl_mk_dnscache() inits a new DNS cache and returns success/failure. + * Curl_init_dnscache() inits a new DNS cache. */ -int Curl_mk_dnscache(struct Curl_hash *hash) +void Curl_init_dnscache(struct Curl_hash *hash) { - return Curl_hash_init(hash, 7, Curl_hash_str, Curl_str_key_compare, - freednsentry); + Curl_hash_init(hash, 7, Curl_hash_str, Curl_str_key_compare, + freednsentry); } /* diff --git a/lib/hostip.h b/lib/hostip.h index 67a688aeb..1db598184 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -129,8 +129,8 @@ struct Curl_addrinfo *Curl_getaddrinfo(struct Curl_easy *data, void Curl_resolv_unlock(struct Curl_easy *data, struct Curl_dns_entry *dns); -/* init a new dns cache and return success */ -int Curl_mk_dnscache(struct Curl_hash *hash); +/* init a new dns cache */ +void Curl_init_dnscache(struct Curl_hash *hash); /* prune old entries from the DNS cache */ void Curl_hostcache_prune(struct Curl_easy *data); diff --git a/lib/multi.c b/lib/multi.c index 2bda97223..f8dcc63b4 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -281,11 +281,8 @@ static struct Curl_sh_entry *sh_addentry(struct Curl_hash *sh, if(!check) return NULL; /* major failure */ - if(Curl_hash_init(&check->transfers, TRHASH_SIZE, trhash, - trhash_compare, trhash_dtor)) { - free(check); - return NULL; - } + Curl_hash_init(&check->transfers, TRHASH_SIZE, trhash, trhash_compare, + trhash_dtor); /* make/add new hash entry */ if(!Curl_hash_add(sh, (char *)&s, sizeof(curl_socket_t), check)) { @@ -352,10 +349,10 @@ static size_t hash_fd(void *key, size_t key_length, size_t slots_num) * per call." * */ -static int sh_init(struct Curl_hash *hash, int hashsize) +static void sh_init(struct Curl_hash *hash, int hashsize) { - return Curl_hash_init(hash, hashsize, hash_fd, fd_key_compare, - sh_freeentry); + Curl_hash_init(hash, hashsize, hash_fd, fd_key_compare, + sh_freeentry); } /* @@ -382,11 +379,9 @@ struct Curl_multi *Curl_multi_handle(int hashsize, /* socket hash */ multi->magic = CURL_MULTI_HANDLE; - if(Curl_mk_dnscache(&multi->hostcache)) - goto error; + Curl_init_dnscache(&multi->hostcache); - if(sh_init(&multi->sockhash, hashsize)) - goto error; + sh_init(&multi->sockhash, hashsize); if(Curl_conncache_init(&multi->conn_cache, chashsize)) goto error; diff --git a/lib/share.c b/lib/share.c index 9c43c8f70..403563fdd 100644 --- a/lib/share.c +++ b/lib/share.c @@ -39,11 +39,7 @@ curl_share_init(void) if(share) { share->magic = CURL_GOOD_SHARE; share->specifier |= (1<hostcache)) { - free(share); - return NULL; - } + Curl_init_dnscache(&share->hostcache); } return share; diff --git a/tests/unit/unit1305.c b/tests/unit/unit1305.c index b0e8eda5e..8e551c790 100644 --- a/tests/unit/unit1305.c +++ b/tests/unit/unit1305.c @@ -46,19 +46,13 @@ static struct Curl_dns_entry *data_node; static CURLcode unit_setup(void) { - int rc; data = curl_easy_init(); if(!data) { curl_global_cleanup(); return CURLE_OUT_OF_MEMORY; } - rc = Curl_mk_dnscache(&hp); - if(rc) { - curl_easy_cleanup(data); - curl_global_cleanup(); - return CURLE_OUT_OF_MEMORY; - } + Curl_init_dnscache(&hp); return CURLE_OK; } diff --git a/tests/unit/unit1602.c b/tests/unit/unit1602.c index ee6acacc5..bb6e504c4 100644 --- a/tests/unit/unit1602.c +++ b/tests/unit/unit1602.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2021, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -38,8 +38,9 @@ static void mydtor(void *p) static CURLcode unit_setup(void) { - return Curl_hash_init(&hash_static, 7, Curl_hash_str, - Curl_str_key_compare, mydtor); + Curl_hash_init(&hash_static, 7, Curl_hash_str, + Curl_str_key_compare, mydtor); + return CURLE_OK; } static void unit_stop(void) diff --git a/tests/unit/unit1603.c b/tests/unit/unit1603.c index f9170ef99..86641d61f 100644 --- a/tests/unit/unit1603.c +++ b/tests/unit/unit1603.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 2015 - 2020, Daniel Stenberg, , et al. + * Copyright (C) 2015 - 2021, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -39,8 +39,9 @@ static void mydtor(void *p) static CURLcode unit_setup(void) { - return Curl_hash_init(&hash_static, slots, Curl_hash_str, - Curl_str_key_compare, mydtor); + Curl_hash_init(&hash_static, slots, Curl_hash_str, + Curl_str_key_compare, mydtor); + return CURLE_OK; } static void unit_stop(void) -- cgit v1.2.1