From ce9908cb4ebee10ee1ba934f8eb32c26b0997898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 8 Mar 2022 11:22:55 +0100 Subject: [PATCH] Make isc_ht_init() and isc_ht_iter_create() return void Previously, the function(s) in the commit subject could fail for various reasons - mostly allocation failures, or other functions returning different return code than ISC_R_SUCCESS. Now, the aforementioned function(s) cannot ever fail and they would always return ISC_R_SUCCESS. Change the function(s) to return void and remove the extra checks in the code that uses them. (cherry picked from commit 8fa27365ec8ea47b498ea64a9b72553c0b662b6b) --- bin/plugins/filter-a.c | 4 +- bin/plugins/filter-aaaa.c | 4 +- bin/tests/system/hooks/driver/test-async.c | 10 +-- lib/dns/catz.c | 82 ++++++---------------- lib/dns/rpz.c | 29 +------- lib/isc/ht.c | 7 +- lib/isc/include/isc/ht.h | 13 ++-- lib/isc/tests/ht_test.c | 9 +-- lib/isc/tls.c | 4 +- 9 files changed, 40 insertions(+), 122 deletions(-) diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 94a5aeb2b1..2ba52df081 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -330,7 +330,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, isc_log_t *lctx, void *actx, ns_hooktable_t *hooktable, void **instp) { filter_instance_t *inst = NULL; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; isc_log_write(lctx, NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, @@ -347,7 +347,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, cfg_line, mctx, lctx, actx)); } - CHECK(isc_ht_init(&inst->ht, mctx, 16)); + isc_ht_init(&inst->ht, mctx, 16); isc_mutex_init(&inst->hlock); /* diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index 85d5029b0c..7f6fac12ac 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -333,7 +333,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, isc_log_t *lctx, void *actx, ns_hooktable_t *hooktable, void **instp) { filter_instance_t *inst = NULL; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; isc_log_write(lctx, NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, @@ -350,7 +350,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, cfg_line, mctx, lctx, actx)); } - CHECK(isc_ht_init(&inst->ht, mctx, 16)); + isc_ht_init(&inst->ht, mctx, 16); isc_mutex_init(&inst->hlock); /* diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c index 8fd98db7f2..577d5de6f8 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/bin/tests/system/hooks/driver/test-async.c @@ -129,7 +129,6 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, isc_log_t *lctx, void *actx, ns_hooktable_t *hooktable, void **instp) { async_instance_t *inst = NULL; - isc_result_t result; UNUSED(parameters); UNUSED(cfg); @@ -144,7 +143,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, *inst = (async_instance_t){ .mctx = NULL }; isc_mem_attach(mctx, &inst->mctx); - CHECK(isc_ht_init(&inst->ht, mctx, 16)); + isc_ht_init(&inst->ht, mctx, 16); isc_mutex_init(&inst->hlock); /* @@ -155,13 +154,6 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, *instp = inst; return (ISC_R_SUCCESS); - -cleanup: - if (result != ISC_R_SUCCESS) { - plugin_destroy((void **)&inst); - } - - return (result); } isc_result_t diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 02c805318a..7a906e12cb 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -392,39 +392,21 @@ dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone) { dns_name_format(&target->name, czname, DNS_NAME_FORMATSIZE); - result = isc_ht_init(&toadd, target->catzs->mctx, 16); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_init(&toadd, target->catzs->mctx, 16); - result = isc_ht_init(&tomod, target->catzs->mctx, 16); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_init(&tomod, target->catzs->mctx, 16); - result = isc_ht_iter_create(newzone->entries, &iter1); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_iter_create(newzone->entries, &iter1); - result = isc_ht_iter_create(target->entries, &iter2); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_iter_create(target->entries, &iter2); /* * We can create those iterators now, even though toadd and tomod are * empty */ - result = isc_ht_iter_create(toadd, &iteradd); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_iter_create(toadd, &iteradd); - result = isc_ht_iter_create(tomod, &itermod); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_ht_iter_create(tomod, &itermod); /* * First - walk the new zone and find all nodes that are not in the @@ -572,25 +554,11 @@ dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone) { result = ISC_R_SUCCESS; -cleanup: - if (iter1 != NULL) { - isc_ht_iter_destroy(&iter1); - } - if (iter2 != NULL) { - isc_ht_iter_destroy(&iter2); - } - if (iteradd != NULL) { - isc_ht_iter_destroy(&iteradd); - } - if (itermod != NULL) { - isc_ht_iter_destroy(&itermod); - } - if (toadd != NULL) { - isc_ht_destroy(&toadd); - } - if (tomod != NULL) { - isc_ht_destroy(&tomod); - } + isc_ht_iter_destroy(&iteradd); + isc_ht_iter_destroy(&itermod); + isc_ht_destroy(&toadd); + isc_ht_destroy(&tomod); + return (result); } @@ -611,10 +579,7 @@ dns_catz_new_zones(dns_catz_zones_t **catzsp, dns_catz_zonemodmethods_t *zmm, isc_refcount_init(&new_zones->refs, 1); - result = isc_ht_init(&new_zones->zones, mctx, 4); - if (result != ISC_R_SUCCESS) { - goto cleanup_refcount; - } + isc_ht_init(&new_zones->zones, mctx, 4); isc_mem_attach(mctx, &new_zones->mctx); new_zones->zmm = zmm; @@ -632,7 +597,6 @@ dns_catz_new_zones(dns_catz_zones_t **catzsp, dns_catz_zonemodmethods_t *zmm, cleanup_ht: isc_ht_destroy(&new_zones->zones); -cleanup_refcount: isc_refcount_destroy(&new_zones->refs); isc_mutex_destroy(&new_zones->lock); isc_mem_put(mctx, new_zones, sizeof(*new_zones)); @@ -667,10 +631,7 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep, dns_name_init(&new_zone->name, NULL); dns_name_dup(name, catzs->mctx, &new_zone->name); - result = isc_ht_init(&new_zone->entries, catzs->mctx, 4); - if (result != ISC_R_SUCCESS) { - goto cleanup_name; - } + isc_ht_init(&new_zone->entries, catzs->mctx, 4); new_zone->updatetimer = NULL; result = isc_timer_create(catzs->timermgr, isc_timertype_inactive, NULL, @@ -700,7 +661,6 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep, cleanup_ht: isc_ht_destroy(&new_zone->entries); -cleanup_name: dns_name_free(&new_zone->name, catzs->mctx); isc_mem_put(catzs->mctx, new_zone, sizeof(*new_zone)); @@ -799,8 +759,7 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) { if (zone->entries != NULL) { isc_ht_iter_t *iter = NULL; isc_result_t result; - result = isc_ht_iter_create(zone->entries, &iter); - INSIST(result == ISC_R_SUCCESS); + isc_ht_iter_create(zone->entries, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS; result = isc_ht_iter_delcurrent_next(iter)) @@ -856,8 +815,7 @@ dns_catz_catzs_detach(dns_catz_zones_t **catzsp) { if (catzs->zones != NULL) { isc_ht_iter_t *iter = NULL; isc_result_t result; - result = isc_ht_iter_create(catzs->zones, &iter); - INSIST(result == ISC_R_SUCCESS); + isc_ht_iter_create(catzs->zones, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS;) { dns_catz_zone_t *zone = NULL; @@ -2010,8 +1968,7 @@ dns_catz_prereconfig(dns_catz_zones_t *catzs) { REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); - result = isc_ht_iter_create(catzs->zones, &iter); - INSIST(result == ISC_R_SUCCESS); + isc_ht_iter_create(catzs->zones, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS; result = isc_ht_iter_next(iter)) { @@ -2032,8 +1989,7 @@ dns_catz_postreconfig(dns_catz_zones_t *catzs) { REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); LOCK(&catzs->lock); - result = isc_ht_iter_create(catzs->zones, &iter); - INSIST(result == ISC_R_SUCCESS); + isc_ht_iter_create(catzs->zones, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS;) { dns_catz_zone_t *zone = NULL; @@ -2072,5 +2028,7 @@ dns_catz_postreconfig(dns_catz_zones_t *catzs) { isc_result_t dns_catz_get_iterator(dns_catz_zone_t *catz, isc_ht_iter_t **itp) { REQUIRE(DNS_CATZ_ZONE_VALID(catz)); - return (isc_ht_iter_create(catz->entries, itp)); + isc_ht_iter_create(catz->entries, itp); + + return (ISC_R_SUCCESS); } diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index f0cfbc6b27..3b54ca6ca3 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -1540,10 +1540,7 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { * simplifies update_from_db */ - result = isc_ht_init(&zone->nodes, rpzs->mctx, 1); - if (result != ISC_R_SUCCESS) { - goto cleanup_ht; - } + isc_ht_init(&zone->nodes, rpzs->mctx, 1); dns_name_init(&zone->origin, NULL); dns_name_init(&zone->client_ip, NULL); @@ -1577,9 +1574,6 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { return (ISC_R_SUCCESS); -cleanup_ht: - isc_timer_detach(&zone->updatetimer); - cleanup_timer: isc_refcount_decrementz(&zone->refs); isc_refcount_destroy(&zone->refs); @@ -1723,14 +1717,7 @@ setup_update(dns_rpz_zone_t *rpz) { ISC_LOG_DEBUG(1), "rpz: %s: using hashtable size %d", domain, hashsize); - result = isc_ht_init(&rpz->newnodes, rpz->rpzs->mctx, hashsize); - if (result != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, - "rpz: %s: failed to initialize hashtable - %s", - domain, isc_result_totext(result)); - goto cleanup; - } + isc_ht_init(&rpz->newnodes, rpz->rpzs->mctx, hashsize); result = dns_db_createiterator(rpz->updb, DNS_DB_NONSEC3, &rpz->updbit); if (result != ISC_R_SUCCESS) { @@ -1837,17 +1824,7 @@ cleanup_quantum(isc_task_t *task, isc_event_t *event) { * Iterate over old ht with existing nodes deleted to * delete deleted nodes from RPZ */ - result = isc_ht_iter_create(rpz->nodes, &iter); - if (result != ISC_R_SUCCESS) { - dns_name_format(&rpz->origin, domain, - DNS_NAME_FORMATSIZE); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, - "rpz: %s: failed to create HT " - "iterator - %s", - domain, isc_result_totext(result)); - goto cleanup; - } + isc_ht_iter_create(rpz->nodes, &iter); } name = dns_fixedname_initname(&fname); diff --git a/lib/isc/ht.c b/lib/isc/ht.c index 34392322a4..d67f91b394 100644 --- a/lib/isc/ht.c +++ b/lib/isc/ht.c @@ -49,7 +49,7 @@ struct isc_ht_iter { isc_ht_node_t *cur; }; -isc_result_t +void isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits) { isc_ht_t *ht = NULL; size_t i; @@ -76,7 +76,6 @@ isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits) { ht->magic = ISC_HT_MAGIC; *htp = ht; - return (ISC_R_SUCCESS); } void @@ -201,7 +200,7 @@ isc_ht_delete(isc_ht_t *ht, const unsigned char *key, uint32_t keysize) { return (ISC_R_NOTFOUND); } -isc_result_t +void isc_ht_iter_create(isc_ht_t *ht, isc_ht_iter_t **itp) { isc_ht_iter_t *it; @@ -215,8 +214,6 @@ isc_ht_iter_create(isc_ht_t *ht, isc_ht_iter_t **itp) { it->cur = NULL; *itp = it; - - return (ISC_R_SUCCESS); } void diff --git a/lib/isc/include/isc/ht.h b/lib/isc/include/isc/ht.h index d8eda7485f..1410a4a810 100644 --- a/lib/isc/include/isc/ht.h +++ b/lib/isc/include/isc/ht.h @@ -32,11 +32,8 @@ typedef struct isc_ht_iter isc_ht_iter_t; *\li 'mctx' is a valid memory context. *\li 'bits' >=1 and 'bits' <=32 * - * Returns: - *\li #ISC_R_NOMEMORY -- not enough memory to create pool - *\li #ISC_R_SUCCESS -- all is well. */ -isc_result_t +void isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits); /*% @@ -101,7 +98,7 @@ isc_ht_delete(isc_ht_t *ht, const unsigned char *key, uint32_t keysize); *\li 'ht' is a valid hashtable *\li 'itp' is non NULL and '*itp' is NULL. */ -isc_result_t +void isc_ht_iter_create(isc_ht_t *ht, isc_ht_iter_t **itp); /*% @@ -120,7 +117,7 @@ isc_ht_iter_destroy(isc_ht_iter_t **itp); *\li 'it' is non NULL. * * Returns: - * \li #ISC_R_SUCCESS -- success + * \li #ISC_R_SUCCESS -- success * \li #ISC_R_NOMORE -- no data in the hashtable */ isc_result_t @@ -133,7 +130,7 @@ isc_ht_iter_first(isc_ht_iter_t *it); *\li 'it' is non NULL. * * Returns: - * \li #ISC_R_SUCCESS -- success + * \li #ISC_R_SUCCESS -- success * \li #ISC_R_NOMORE -- end of hashtable reached */ isc_result_t @@ -146,7 +143,7 @@ isc_ht_iter_next(isc_ht_iter_t *it); *\li 'it' is non NULL. * * Returns: - * \li #ISC_R_SUCCESS -- success + * \li #ISC_R_SUCCESS -- success * \li #ISC_R_NOMORE -- end of hashtable reached */ isc_result_t diff --git a/lib/isc/tests/ht_test.c b/lib/isc/tests/ht_test.c index 1c1a4b76e5..89f9794bc9 100644 --- a/lib/isc/tests/ht_test.c +++ b/lib/isc/tests/ht_test.c @@ -61,8 +61,7 @@ test_ht_full(int bits, uintptr_t count) { isc_result_t result; uintptr_t i; - result = isc_ht_init(&ht, test_mctx, bits); - assert_int_equal(result, ISC_R_SUCCESS); + isc_ht_init(&ht, test_mctx, bits); assert_non_null(ht); for (i = 1; i < count; i++) { @@ -207,8 +206,7 @@ test_ht_iterator(void) { unsigned char key[16]; size_t tksize; - result = isc_ht_init(&ht, test_mctx, 16); - assert_int_equal(result, ISC_R_SUCCESS); + isc_ht_init(&ht, test_mctx, 16); assert_non_null(ht); for (i = 1; i <= count; i++) { /* @@ -222,8 +220,7 @@ test_ht_iterator(void) { } walked = 0; - result = isc_ht_iter_create(ht, &iter); - assert_int_equal(result, ISC_R_SUCCESS); + isc_ht_iter_create(ht, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS; result = isc_ht_iter_next(iter)) diff --git a/lib/isc/tls.c b/lib/isc/tls.c index c15e596911..349e026ea0 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -940,7 +940,7 @@ isc_tlsctx_cache_new(isc_mem_t *mctx) { isc_refcount_init(&nc->references, 1); isc_mem_attach(mctx, &nc->mctx); - RUNTIME_CHECK(isc_ht_init(&nc->data, mctx, 5) == ISC_R_SUCCESS); + isc_ht_init(&nc->data, mctx, 5); isc_rwlock_init(&nc->rwlock, 0, 0); return (nc); @@ -980,7 +980,7 @@ tlsctx_cache_destroy(isc_tlsctx_cache_t *cache) { isc_refcount_destroy(&cache->references); - RUNTIME_CHECK(isc_ht_iter_create(cache->data, &it) == ISC_R_SUCCESS); + isc_ht_iter_create(cache->data, &it); for (result = isc_ht_iter_first(it); result == ISC_R_SUCCESS; result = isc_ht_iter_delcurrent_next(it)) {