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.

diff --git a/lib/dns/client.c b/lib/dns/client.c
index cbdfc16badcc34300eb4ca6ca3e3987e1be9f270..879915ba28cd56996169a877666457f23b9377d3 100644 (file)
--- 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 e99e21c76338bac54ce339c4213b8320cf870c63..eaf948643e3797cdf800a8130233d25b8e6de629 100644 (file)
--- 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 0c70c78d1a10d22bf89ee3f8cf5c0469e0f8d74f..8d55155914375cd30752fc06ffd8e5ac176cd1a0 100644 (file)
--- 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 68fbf0ade4231d4d71f6a954f84472c5fcd3362b..55ebb314e1d7b53cbb758c60615a42754610e714 100644 (file)
--- a/lib/isc/include/isc/counter.h
+++ b/lib/isc/include/isc/counter.h
@@ -31,13 +31,14 @@
  ***/
 
 #include <isc/mutex.h>
+#include <isc/refcount.h>
 #include <isc/types.h>
 
 /*****
 ***** 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 e2c4a67c51661aada90e39ef37fa4d631c0894ba..fe57ada093ed169b3b0589f07de65a8418bdf14d 100644 (file)
--- 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 dc8dc50f6ad3554b217c6996fe993b77e4562e6a..594d948f479fcdd08b6ad0cdf36e55f7a4a2ede1 100644 (file)
--- 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);
 }