From 77ec2a6c22e8981e58f55b01457ffdba561703b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 19 Feb 2025 06:49:38 +0100 Subject: [PATCH] Cleanup the isc_counter unit The isc_counter_create() doesn't need the return value (it was always ISC_R_SUCCESS), use the macros to implement the reference counting, little style cleanup, and expand the unit test. --- lib/dns/client.c | 5 +--- lib/dns/resolver.c | 7 +---- lib/isc/counter.c | 54 ++++++++++------------------------- lib/isc/include/isc/counter.h | 16 ++--------- lib/ns/query.c | 9 ++---- tests/isc/counter_test.c | 21 ++++++++------ 6 files changed, 34 insertions(+), 78 deletions(-) diff --git a/lib/dns/client.c b/lib/dns/client.c index cbdfc16bad..879915ba28 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -939,10 +939,7 @@ startresolve(dns_client_t *client, const dns_name_t *name, rctx->magic = RCTX_MAGIC; isc_refcount_increment(&client->references); - result = isc_counter_create(mctx, client->max_queries, &rctx->qc); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + isc_counter_create(mctx, client->max_queries, &rctx->qc); ISC_LIST_APPEND(client->resctxs, rctx, link); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index e99e21c763..eaf948643e 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4595,11 +4595,7 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, "fctx %p(%s): attached to counter %p (%d)", fctx, fctx->info, fctx->qc, isc_counter_used(fctx->qc)); } else { - result = isc_counter_create(fctx->mctx, res->maxqueries, - &fctx->qc); - if (result != ISC_R_SUCCESS) { - goto cleanup_fetch; - } + isc_counter_create(fctx->mctx, res->maxqueries, &fctx->qc); isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(9), "fctx %p(%s): created counter %p", fctx, @@ -4827,7 +4823,6 @@ cleanup_nameservers: isc_counter_detach(&fctx->gqc); } -cleanup_fetch: dns_resolver_detach(&fctx->res); isc_mem_putanddetach(&fctx->mctx, fctx, sizeof(*fctx)); diff --git a/lib/isc/counter.c b/lib/isc/counter.c index 0c70c78d1a..8d55155914 100644 --- a/lib/isc/counter.c +++ b/lib/isc/counter.c @@ -34,30 +34,26 @@ struct isc_counter { atomic_uint_fast32_t used; }; -isc_result_t +void isc_counter_create(isc_mem_t *mctx, int limit, isc_counter_t **counterp) { - isc_counter_t *counter; - REQUIRE(counterp != NULL && *counterp == NULL); - counter = isc_mem_get(mctx, sizeof(*counter)); + isc_counter_t *counter = isc_mem_get(mctx, sizeof(*counter)); + *counter = (isc_counter_t){ + .magic = COUNTER_MAGIC, + .references = 1, + .limit = limit, + }; - counter->mctx = NULL; isc_mem_attach(mctx, &counter->mctx); - isc_refcount_init(&counter->references, 1); - atomic_init(&counter->limit, limit); - atomic_init(&counter->used, 0); - - counter->magic = COUNTER_MAGIC; *counterp = counter; - return ISC_R_SUCCESS; } isc_result_t isc_counter_increment(isc_counter_t *counter) { - uint32_t used = atomic_fetch_add_relaxed(&counter->used, 1) + 1; - uint32_t limit = atomic_load_acquire(&counter->limit); + uint_fast32_t used = atomic_fetch_add_relaxed(&counter->used, 1) + 1; + uint_fast32_t limit = atomic_load_acquire(&counter->limit); if (limit != 0 && used >= limit) { return ISC_R_QUOTA; @@ -70,14 +66,14 @@ unsigned int isc_counter_used(isc_counter_t *counter) { REQUIRE(VALID_COUNTER(counter)); - return atomic_load_acquire(&counter->used); + return atomic_load_relaxed(&counter->used); } void isc_counter_setlimit(isc_counter_t *counter, int limit) { REQUIRE(VALID_COUNTER(counter)); - atomic_store(&counter->limit, limit); + atomic_store_release(&counter->limit, limit); } unsigned int @@ -87,33 +83,13 @@ isc_counter_getlimit(isc_counter_t *counter) { return atomic_load_acquire(&counter->limit); } -void -isc_counter_attach(isc_counter_t *source, isc_counter_t **targetp) { - REQUIRE(VALID_COUNTER(source)); - REQUIRE(targetp != NULL && *targetp == NULL); - - isc_refcount_increment(&source->references); - - *targetp = source; -} - static void -destroy(isc_counter_t *counter) { +isc__counter_destroy(isc_counter_t *counter) { + REQUIRE(VALID_COUNTER(counter)); + isc_refcount_destroy(&counter->references); counter->magic = 0; isc_mem_putanddetach(&counter->mctx, counter, sizeof(*counter)); } -void -isc_counter_detach(isc_counter_t **counterp) { - isc_counter_t *counter; - - REQUIRE(counterp != NULL && *counterp != NULL); - counter = *counterp; - *counterp = NULL; - REQUIRE(VALID_COUNTER(counter)); - - if (isc_refcount_decrement(&counter->references) == 1) { - destroy(counter); - } -} +ISC_REFCOUNT_IMPL(isc_counter, isc__counter_destroy); diff --git a/lib/isc/include/isc/counter.h b/lib/isc/include/isc/counter.h index 68fbf0ade4..55ebb314e1 100644 --- a/lib/isc/include/isc/counter.h +++ b/lib/isc/include/isc/counter.h @@ -31,13 +31,14 @@ ***/ #include +#include #include /***** ***** Types. *****/ -isc_result_t +void isc_counter_create(isc_mem_t *mctx, int limit, isc_counter_t **counterp); /*%< * Allocate and initialize a counter object. @@ -71,15 +72,4 @@ isc_counter_getlimit(isc_counter_t *counter); * Get the counter limit. */ -void -isc_counter_attach(isc_counter_t *source, isc_counter_t **targetp); -/*%< - * Attach to a counter object, increasing its reference counter. - */ - -void -isc_counter_detach(isc_counter_t **counterp); -/*%< - * Detach (and destroy if reference counter has dropped to zero) - * a counter object. - */ +ISC_REFCOUNT_DECL(isc_counter); diff --git a/lib/ns/query.c b/lib/ns/query.c index e2c4a67c51..fe57ada093 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -11969,13 +11969,8 @@ ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) { /* * Start global outgoing query count. */ - result = isc_counter_create(client->manager->mctx, - client->view->max_queries, - &client->query.qc); - if (result != ISC_R_SUCCESS) { - query_next(client, result); - return; - } + isc_counter_create(client->manager->mctx, client->view->max_queries, + &client->query.qc); query_setup(client, qtype); } diff --git a/tests/isc/counter_test.c b/tests/isc/counter_test.c index dc8dc50f6a..594d948f47 100644 --- a/tests/isc/counter_test.c +++ b/tests/isc/counter_test.c @@ -32,29 +32,32 @@ ISC_RUN_TEST_IMPL(isc_counter) { isc_result_t result; isc_counter_t *counter = NULL; - int i; - UNUSED(state); + isc_counter_create(mctx, 0, &counter); - result = isc_counter_create(mctx, 0, &counter); - assert_int_equal(result, ISC_R_SUCCESS); - - for (i = 0; i < 10; i++) { + for (size_t i = 0; i < 10; i++) { result = isc_counter_increment(counter); assert_int_equal(result, ISC_R_SUCCESS); } assert_int_equal(isc_counter_used(counter), 10); - isc_counter_setlimit(counter, 15); - for (i = 0; i < 10; i++) { + isc_counter_setlimit(counter, 10); + + for (size_t i = 0; i < 5; i++) { + result = isc_counter_increment(counter); + assert_int_equal(result, ISC_R_QUOTA); + } + + isc_counter_setlimit(counter, 20); + for (size_t i = 0; i < 10; i++) { result = isc_counter_increment(counter); if (result != ISC_R_SUCCESS) { break; } } - assert_int_equal(isc_counter_used(counter), 15); + assert_int_equal(isc_counter_used(counter), 20); isc_counter_detach(&counter); }