chg:nil: 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.

Merge branch 'ondrej/cleanup-isc_counter-unit' into 'main'

See merge request isc-projects/bind9!10126

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);
+	isc_counter_create(mctx, client->max_queries, &rctx->qc);
-	if (result != ISC_R_SUCCESS) {
-		goto cleanup;
-	}
 
 	ISC_LIST_APPEND(client->resctxs, rctx, link);
 

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,
+		isc_counter_create(fctx->mctx, res->maxqueries, &fctx->qc);
-					    &fctx->qc);
-		if (result != ISC_R_SUCCESS) {
-			goto cleanup_fetch;
-		}
 		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));
 

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;
+	uint_fast32_t used = atomic_fetch_add_relaxed(&counter->used, 1) + 1;
-	uint32_t limit = atomic_load_acquire(&counter->limit);
+	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_REFCOUNT_IMPL(isc_counter, isc__counter_destroy);
-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);
-	}
-}

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_REFCOUNT_DECL(isc_counter);
-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.
- */

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,
+	isc_counter_create(client->manager->mctx, client->view->max_queries,
-				    client->view->max_queries,
+			   &client->query.qc);
-				    &client->query.qc);
-	if (result != ISC_R_SUCCESS) {
-		query_next(client, result);
-		return;
-	}
 
 	query_setup(client, qtype);
 }

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);
+	for (size_t i = 0; i < 10; i++) {
-	assert_int_equal(result, ISC_R_SUCCESS);
-
-	for (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);
+	isc_counter_setlimit(counter, 10);
-	for (i = 0; i < 10; i++) {
+
+	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);
 }