Catalog zones: use iterators instead of isc_ht_walk [RT #42529]

.gitignore

@@ -12,6 +12,8 @@ autom4te.cache/
 *.so
 *.a
 *.la
+*.gcno
+*.gcda
 *_test
 *-symtbl.c
 timestamp

lib/dns/catz.c

@@ -101,7 +101,7 @@ dns_catz_options_init(dns_catz_options_t *options) {
 
 void
 dns_catz_options_free(dns_catz_options_t *options, isc_mem_t *mctx) {
-	if (options->masters.count > 0)
+	if (options->masters.count != 0)
 		dns_ipkeylist_clear(mctx, &options->masters);
 	if (options->zonedir != NULL) {
 		isc_mem_free(mctx, options->zonedir);
@@ -270,134 +270,126 @@ dns_catz_zone_resetdefoptions(dns_catz_zone_t *zone) {
 	dns_catz_options_init(&zone->defoptions);
 }
 
-static isc_result_t
-newzonewalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-	    void *data)
-{
-	isc_result_t result;
-	dns_catz_zone_t *zone = udata;
-	dns_catz_entry_t *nentry = (dns_catz_entry_t *) data;
-	dns_catz_entry_t *oentry;
-	char cznamebuf[DNS_NAME_FORMATSIZE];
-	char znamebuf[DNS_NAME_FORMATSIZE];
-	isc_buffer_t czname;
-	isc_buffer_t zname;
-
-	REQUIRE(zone != NULL);
-	REQUIRE(zone->catzs->view != NULL);
-
-	dns_catz_options_setdefault(zone->catzs->mctx, &zone->zoneoptions,
-				    &nentry->opts);
-	isc_buffer_init(&czname, cznamebuf, DNS_NAME_FORMATSIZE);
-	isc_buffer_init(&zname, znamebuf, DNS_NAME_FORMATSIZE);
-	dns_name_totext(&zone->name, ISC_TRUE, &czname);
-	isc_buffer_putuint8(&czname, 0);
-	dns_name_totext(&nentry->name, ISC_TRUE, &zname);
-	isc_buffer_putuint8(&zname, 0);
-
-	result = isc_ht_find(zone->entries, key, keysize, (void **) &oentry);
-	if (result != ISC_R_SUCCESS) {
-		result = zone->catzs->zmm->addzone(nentry, zone,
-						   zone->catzs->view,
-						   zone->catzs->taskmgr,
-						   zone->catzs->zmm->udata);
-		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
-			      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
-			      "catz: adding zone '%s' from catalog '%s' - %s",
-			      znamebuf, cznamebuf, isc_result_totext(result));
-		return (ISC_R_SUCCESS);
-	}
-
-	if (dns_catz_entry_cmp(oentry, nentry) != ISC_TRUE) {
-		result = zone->catzs->zmm->modzone(nentry, zone,
-						   zone->catzs->view,
-						   zone->catzs->taskmgr,
-						   zone->catzs->zmm->udata);
-		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
-			      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
-			      "catz: modifying zone '%s' from catalog "
-			      "'%s' - %s",
-			      znamebuf, cznamebuf,
-			      isc_result_totext(result));
-	}
-
-	dns_catz_entry_detach(zone, &oentry);
-	result = isc_ht_delete(zone->entries, key, keysize);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS);
-
-	return (ISC_R_SUCCESS);
-}
-
-static isc_result_t
-oldzonewalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-	    void *data)
-{
-	isc_result_t result;
-	dns_catz_zone_t *zone = udata;
-	dns_catz_entry_t *entry = (dns_catz_entry_t *) data;
-	char cznamebuf[DNS_NAME_FORMATSIZE];
-	char znamebuf[DNS_NAME_FORMATSIZE];
-	isc_buffer_t czname;
-	isc_buffer_t zname;
-
-	UNUSED(key);
-	UNUSED(keysize);
-
-	REQUIRE(zone != NULL);
-	REQUIRE(zone->catzs->view != NULL);
-
-	isc_buffer_init(&czname, cznamebuf, DNS_NAME_MAXTEXT);
-	isc_buffer_init(&zname, znamebuf, DNS_NAME_MAXTEXT);
-	dns_name_totext(&zone->name, ISC_TRUE, &czname);
-	isc_buffer_putuint8(&czname, 0);
-	dns_name_totext(&entry->name, ISC_TRUE, &zname);
-	isc_buffer_putuint8(&czname, 0);
-	result = zone->catzs->zmm->delzone(entry, zone, zone->catzs->view,
-				  zone->catzs->taskmgr,
-				  zone->catzs->zmm->udata);
-	isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
-		      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
-		      "catz: deleting zone '%s' from catalog '%s' - %s",
-		      znamebuf, cznamebuf, isc_result_totext(result));
-	dns_catz_entry_detach(zone, &entry);
-
-	return (ISC_R_EXISTS);
-}
-
 isc_result_t
 dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone) {
 	isc_result_t result;
-
+	isc_ht_iter_t *iter = NULL;
+	char cznamebuf[DNS_NAME_FORMATSIZE];
+	char znamebuf[DNS_NAME_FORMATSIZE];
+	isc_buffer_t czname;
+	isc_buffer_t zname;
+	dns_catz_zoneop_fn_t addzone, modzone, delzone;
 	REQUIRE(target != NULL);
 	REQUIRE(newzone != NULL);
 
 	/* TODO verify the new zone first! */
 
+	addzone = target->catzs->zmm->addzone;
+	modzone = target->catzs->zmm->modzone;
+	delzone = target->catzs->zmm->delzone;
+
 	/* Copy zoneoptions from newzone into target. */
+
 	dns_catz_options_free(&target->zoneoptions, target->catzs->mctx);
 	dns_catz_options_copy(target->catzs->mctx, &newzone->zoneoptions,
 			      &target->zoneoptions);
 	dns_catz_options_setdefault(target->catzs->mctx, &target->defoptions,
 				    &target->zoneoptions);
 
+	isc_buffer_init(&czname, cznamebuf, DNS_NAME_FORMATSIZE);
+	isc_buffer_init(&zname, znamebuf, DNS_NAME_FORMATSIZE);
+	dns_name_totext(&target->name, ISC_TRUE, &czname);
+	isc_buffer_putuint8(&czname, 0);
+
 	/*
 	 * first - walk the new zone and find all nodes that are not in the
 	 * old zone, or are in both zones and are modified
 	 */
-	result = isc_ht_walk(newzone->entries, newzonewalk, target);
+	result = isc_ht_iter_create(newzone->entries, &iter);
-	/* newzonewalk always returns ISC_R_SUCCESS */
+	INSIST(result == ISC_R_SUCCESS);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS);
+
+	result = isc_ht_iter_first(iter);
+	for (result = isc_ht_iter_first(iter);
+	     result == ISC_R_SUCCESS;
+	     result = isc_ht_iter_next(iter))
+	{
+		dns_catz_entry_t *nentry;
+		dns_catz_entry_t *oentry;
+		unsigned char * key;
+		size_t keysize;
+
+		isc_ht_iter_current(iter, (void **) &nentry);
+		isc_ht_iter_currentkey(iter, &key, &keysize);
+
+		isc_buffer_clear(&zname);
+		dns_name_totext(&nentry->name, ISC_TRUE, &zname);
+		isc_buffer_putuint8(&zname, 0);
+
+		dns_catz_options_setdefault(target->catzs->mctx,
+					    &target->zoneoptions,
+					    &nentry->opts);
+
+		result = isc_ht_find(target->entries, key, keysize,
+				     (void **) &oentry);
+		if (result != ISC_R_SUCCESS) {
+			result = addzone(nentry, target, target->catzs->view,
+				      target->catzs->taskmgr,
+				      target->catzs->zmm->udata);
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
+				      "catz: adding zone '%s' from catalog "
+				      "'%s' - %s",
+				      znamebuf, cznamebuf,
+				      isc_result_totext(result));
+			continue;
+		}
+
+		if (dns_catz_entry_cmp(oentry, nentry) != ISC_TRUE) {
+			result = modzone(nentry, target, target->catzs->view,
+				      target->catzs->taskmgr,
+				      target->catzs->zmm->udata);
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
+				      "catz: modifying zone '%s' from catalog "
+				      "'%s' - %s",
+				      znamebuf, cznamebuf,
+				      isc_result_totext(result));
+		}
+
+		dns_catz_entry_detach(target, &oentry);
+		result = isc_ht_delete(target->entries, key, keysize);
+		RUNTIME_CHECK(result == ISC_R_SUCCESS);
+	}
+	RUNTIME_CHECK(result == ISC_R_NOMORE);
+	isc_ht_iter_destroy(&iter);
 
 	/*
 	 * then - walk the old zone; only deleted entries should remain
-	 * return (ISC_R_SUCCESS);
 	 */
-	result = isc_ht_walk(target->entries, oldzonewalk, target);
+	result = isc_ht_iter_create(target->entries, &iter);
-	/*
+	INSIST(result == ISC_R_SUCCESS);
-	 * oldzonewalk always returns ISC_R_EXISTS, so walk should return
+
-	 * ISC_R_SUCCESS
+	for (result = isc_ht_iter_first(iter);
-	 */
+	     result == ISC_R_SUCCESS;
-	RUNTIME_CHECK(result == ISC_R_SUCCESS);
+	     result = isc_ht_iter_delcurrent_next(iter))
+	{
+		dns_catz_entry_t *entry;
+		isc_ht_iter_current(iter, (void **) &entry);
+
+		isc_buffer_clear(&zname);
+		dns_name_totext(&entry->name, ISC_TRUE, &zname);
+		isc_buffer_putuint8(&zname, 0);
+		result = delzone(entry, target, target->catzs->view,
+			      target->catzs->taskmgr,
+			      target->catzs->zmm->udata);
+		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+			      DNS_LOGMODULE_MASTER, ISC_LOG_INFO,
+			      "catz: deleting zone '%s' from catalog '%s' - %s",
+			      znamebuf, cznamebuf, isc_result_totext(result));
+		dns_catz_entry_detach(target, &entry);
+	}
+	RUNTIME_CHECK(result == ISC_R_NOMORE);
+	isc_ht_iter_destroy(&iter);
 
 	/* at this moment target->entries has to be be empty */
 	INSIST(isc_ht_count(target->entries) == 0);
@@ -467,6 +459,7 @@ dns_catz_catzs_set_view(dns_catz_zones_t *catzs, dns_view_t *view) {
 	REQUIRE(view != NULL);
 	/* either it's a new one or it's being reconfigured */
 	REQUIRE(catzs->view == NULL || !strcmp(catzs->view->name, view->name));
+
 	catzs->view = view;
 }
 
@@ -588,21 +581,6 @@ dns_catz_catzs_attach(dns_catz_zones_t *catzs, dns_catz_zones_t **catzsp) {
 	*catzsp = catzs;
 }
 
-static isc_result_t
-freewalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-	 void *data)
-{
-	dns_catz_zone_t *zone = (dns_catz_zone_t *) udata;
-	dns_catz_entry_t *entry = (dns_catz_entry_t *) data;
-
-	UNUSED(key);
-	UNUSED(keysize);
-
-	dns_catz_entry_detach(zone, &entry);
-
-	return (ISC_R_EXISTS);
-}
-
 void
 dns_catz_zone_attach(dns_catz_zone_t *zone, dns_catz_zone_t **zonep) {
 	REQUIRE(zonep != NULL && *zonep == NULL);
@@ -613,6 +591,7 @@ dns_catz_zone_attach(dns_catz_zone_t *zone, dns_catz_zone_t **zonep) {
 void
 dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 	dns_catz_zone_t *zone;
+	isc_ht_iter_t *iter = NULL;
 	isc_mem_t *mctx;
 	unsigned int refs;
 
@@ -625,13 +604,18 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 		if (zone->entries != NULL) {
 			isc_result_t result;
 
-			/*
+			result = isc_ht_iter_create(zone->entries, &iter);
-			 * freewalk always returns ISC_R_EXISTS, triggering
-			 * isc_ht_walk to delete the node.  If isc_ht_walk
-			 * returns an error, it is a critical condition
-			 */
-			result = isc_ht_walk(zone->entries, freewalk, zone);
 			INSIST(result == ISC_R_SUCCESS);
+			for (result = isc_ht_iter_first(iter);
+			     result == ISC_R_SUCCESS;
+			     result = isc_ht_iter_delcurrent_next(iter))
+			{
+				dns_catz_entry_t *entry;
+				isc_ht_iter_current(iter, (void **) &entry);
+				dns_catz_entry_detach(zone, &entry);
+			}
+			INSIST(result == ISC_R_NOMORE);
+			isc_ht_iter_destroy(&iter);
 
 			/* the hashtable has to be empty now */
 			INSIST(isc_ht_count(zone->entries) == 0);
@@ -648,27 +632,14 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 	}
 }
 
-static isc_result_t
-catzsfreewalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-	      void *data)
-{
-	dns_catz_zones_t *catzs = (dns_catz_zones_t *) udata;
-	dns_catz_zone_t *zone = (dns_catz_zone_t *) data;
-
-	UNUSED(key);
-	UNUSED(keysize);
-	UNUSED(catzs);
-
-	dns_catz_zone_detach(&zone);
-
-	return (ISC_R_EXISTS);
-}
-
 void
 dns_catz_catzs_detach(dns_catz_zones_t ** catzsp) {
 	dns_catz_zones_t *catzs;
+	isc_ht_iter_t *iter = NULL;
 	isc_result_t result;
 	unsigned int refs;
+	dns_catz_zone_t *zone;
+
 
 	REQUIRE(catzsp != NULL);
 	catzs = *catzsp;
@@ -680,9 +651,17 @@ dns_catz_catzs_detach(dns_catz_zones_t ** catzsp) {
 	if (refs == 0) {
 		DESTROYLOCK(&catzs->lock);
 		if (catzs->zones != NULL) {
-			result = isc_ht_walk(catzs->zones, catzsfreewalk,
+			result = isc_ht_iter_create(catzs->zones, &iter);
-					     catzs);
 			INSIST(result == ISC_R_SUCCESS);
+			for (result = isc_ht_iter_first(iter);
+			     result == ISC_R_SUCCESS;
+			     result = isc_ht_iter_delcurrent_next(iter))
+			{
+				isc_ht_iter_current(iter, (void **) &zone);
+				dns_catz_zone_detach(&zone);
+			}
+			INSIST(result == ISC_R_NOMORE);
+			isc_ht_iter_destroy(&iter);
 			INSIST(isc_ht_count(catzs->zones) == 0);
 			isc_ht_destroy(&catzs->zones);
 		}
@@ -1130,7 +1109,7 @@ dns_catz_generate_masterfilename(dns_catz_zone_t *zone, dns_catz_entry_t *entry,
 	if (tbuf->used > ISC_SHA256_DIGESTSTRINGLENGTH) {
 		isc_sha256_init(&sha256);
 		isc_sha256_update(&sha256, r.base, r.length);
-		/* we can do that because digest string < 2*DNS_NAME */
+		/* we can do that because digest string < 2 * DNS_NAME */
 		isc_sha256_end(&sha256, (char *) r.base);
 		isc_buffer_putstr(*buffer, (char *) r.base);
 	} else {
@@ -1250,7 +1229,7 @@ dns_catz_dbupdate_callback(dns_db_t *db, void *fn_arg) {
 
 	REQUIRE(DNS_DB_VALID(db));
 	REQUIRE(fn_arg != NULL);
-	catzs = (dns_catz_zones_t*) fn_arg;
+	catzs = (dns_catz_zones_t *) fn_arg;
 
 	dns_name_toregion(&db->origin, &r);
 
@@ -1473,70 +1452,66 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) {
 		      "catz: update_from_db: new zone merged");
 }
 
-
-static isc_result_t
-resetactivebitwalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-		   void *data)
-{
-	dns_catz_zone_t *zone = (dns_catz_zone_t *) data;
-
-	UNUSED(udata);
-	UNUSED(key);
-	UNUSED(keysize);
-
-	zone->active = ISC_FALSE;
-	return (ISC_R_SUCCESS);
-}
-
-
 void
 dns_catz_prereconfig(dns_catz_zones_t *catzs) {
-	isc_ht_walk(catzs->zones, resetactivebitwalk, NULL);
-}
-
-static isc_result_t
-postreconfigwalk(void *udata, const unsigned char *key, isc_uint32_t keysize,
-		 void *data)
-{
 	isc_result_t result;
-	dns_catz_zone_t *newzone = NULL;
+	isc_ht_iter_t *iter = NULL;
-	dns_catz_zones_t *catzs = (dns_catz_zones_t *) udata;
+	dns_catz_zone_t *zone;
-	dns_catz_zone_t *zone = (dns_catz_zone_t *) data;
-
-	UNUSED(key);
-	UNUSED(keysize);
 
 	REQUIRE(catzs != NULL);
-	REQUIRE(zone != NULL);
 
-	if (zone->active == ISC_FALSE) {
+	result = isc_ht_iter_create(catzs->zones, &iter);
-		char cname[DNS_NAME_FORMATSIZE];
+	INSIST(result == ISC_R_SUCCESS);
-		dns_name_format(&zone->name, cname, DNS_NAME_FORMATSIZE);
+	for (result = isc_ht_iter_first(iter);
-		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+	     result == ISC_R_SUCCESS;
-			      DNS_LOGMODULE_MASTER,
+	     result = isc_ht_iter_next(iter))
-			      ISC_LOG_WARNING,
+	{
-			      "catz: removing catalog zone %s", cname);
+		isc_ht_iter_current(iter, (void **) &zone);
-
+		zone->active = ISC_FALSE;
-		/* Merge the old zone with an empty one to remove all members */
-		result = dns_catz_new_zone(catzs, &newzone, &zone->name);
-		INSIST(result == ISC_R_SUCCESS);
-		dns_catz_zones_merge(zone, newzone);
-		dns_catz_zone_detach(&newzone);
-
-		/* Make sure that we have an empty catalog zone */
-		INSIST(isc_ht_count(zone->entries) == 0);
-
-		dns_catz_zone_detach(&zone);
-		return (ISC_R_EXISTS);
 	}
-
+	INSIST(result == ISC_R_NOMORE);
-	return (ISC_R_SUCCESS);
+	isc_ht_iter_destroy(&iter);
 }
 
-
 void
 dns_catz_postreconfig(dns_catz_zones_t *catzs) {
-	isc_ht_walk(catzs->zones, postreconfigwalk, catzs);
+	isc_result_t result;
+	dns_catz_zone_t *newzone = NULL;
+	isc_ht_iter_t *iter = NULL;
+	dns_catz_zone_t *zone;
+
+	result = isc_ht_iter_create(catzs->zones, &iter);
+	INSIST(result == ISC_R_SUCCESS);
+	for (result = isc_ht_iter_first(iter);
+	     result == ISC_R_SUCCESS;
+	     result = isc_ht_iter_next(iter))
+	{
+		isc_ht_iter_current(iter, (void **) &zone);
+		if (zone->active == ISC_FALSE) {
+			char cname[DNS_NAME_FORMATSIZE];
+			dns_name_format(&zone->name, cname, DNS_NAME_FORMATSIZE);
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_MASTER,
+				      ISC_LOG_WARNING,
+				      "catz: removing catalog zone %s", cname);
+
+			/*
+			 * Merge the old zone with an empty one to remove
+			 * all members
+			 */
+			result = dns_catz_new_zone(catzs, &newzone,
+						   &zone->name);
+			INSIST(result == ISC_R_SUCCESS);
+			dns_catz_zones_merge(zone, newzone);
+			dns_catz_zone_detach(&newzone);
+
+			/* Make sure that we have an empty catalog zone */
+			INSIST(isc_ht_count(zone->entries) == 0);
+			dns_catz_zone_detach(&zone);
+		}
+	}
+	RUNTIME_CHECK(result == ISC_R_NOMORE);
+	isc_ht_iter_destroy(&iter);
 }
 
 isc_result_t

lib/dns/include/dns/catz.h

@@ -318,13 +318,13 @@ dns_catz_generate_zonecfg(dns_catz_zone_t *zone, dns_catz_entry_t *entry,
 
 /* Methods provided by named to dynamically modify the member zones */
 /* xxxwpk TODO config! */
+typedef isc_result_t (*dns_catz_zoneop_fn_t)(dns_catz_entry_t *entry,
+		dns_catz_zone_t *origin, dns_view_t *view,
+		isc_taskmgr_t *taskmgr, void *udata);
 struct dns_catz_zonemodmethods {
-	isc_result_t (*addzone)(dns_catz_entry_t *entry, dns_catz_zone_t *origin,
+	dns_catz_zoneop_fn_t addzone;
-			dns_view_t *view, isc_taskmgr_t *taskmgr, void *udata);
+	dns_catz_zoneop_fn_t modzone;
-	isc_result_t (*modzone)(dns_catz_entry_t *entry, dns_catz_zone_t *origin,
+	dns_catz_zoneop_fn_t delzone;
-			dns_view_t *view, isc_taskmgr_t *taskmgr, void *udata);
-	isc_result_t (*delzone)(dns_catz_entry_t *entry, dns_catz_zone_t *origin,
-			dns_view_t *view, isc_taskmgr_t *task, void *udata);
 	void * udata;
 };
 

lib/isc/ht.c

@@ -224,9 +224,6 @@ isc_ht_iter_create(isc_ht_t *ht, isc_ht_iter_t **itp) {
 	REQUIRE(ISC_HT_VALID(ht));
 	REQUIRE(itp != NULL && *itp == NULL);
 
-	if (ht->count == 0)
-		return (ISC_R_NOMORE);
-
 	it = isc_mem_get(ht->mctx, sizeof(isc_ht_iter_t));
 	if (it == NULL)
 		return (ISC_R_NOMEMORY);
@@ -288,6 +285,51 @@ isc_ht_iter_next(isc_ht_iter_t *it) {
 	return (ISC_R_SUCCESS);
 }
 
+isc_result_t
+isc_ht_iter_delcurrent_next(isc_ht_iter_t *it) {
+	isc_result_t result = ISC_R_SUCCESS;
+	isc_ht_node_t *to_delete = NULL;
+	isc_ht_node_t *prev = NULL;
+	isc_ht_node_t *node = NULL;
+	isc_uint32_t hash;
+	isc_ht_t *ht;
+	REQUIRE(it != NULL);
+	REQUIRE(it->cur != NULL);
+	to_delete = it->cur;
+	ht = it->ht;
+
+	it->cur = it->cur->next;
+	if (it->cur == NULL) {
+		do {
+		        it->i++;
+		} while (it->i < ht->size && ht->table[it->i] == NULL);
+		if (it->i >= ht->size)
+			result = ISC_R_NOMORE;
+		else
+			it->cur = ht->table[it->i];
+	}
+
+	hash = isc_hash_function(to_delete->key, to_delete->keysize, ISC_TRUE,
+				 NULL);
+	node = ht->table[hash & ht->mask];
+	while (node != to_delete) {
+		prev = node;
+		node = node->next;
+		INSIST(node != NULL);
+	}
+
+	if (prev == NULL) {
+		ht->table[hash & ht->mask] = node->next;
+	} else {
+		prev->next = node->next;
+	}
+	isc_mem_put(ht->mctx, node,
+		    sizeof(isc_ht_node_t) + node->keysize);
+	ht->count--;
+
+	return (result);
+}
+
 void
 isc_ht_iter_current(isc_ht_iter_t *it, void **valuep) {
 	REQUIRE(it != NULL);
@@ -295,48 +337,18 @@ isc_ht_iter_current(isc_ht_iter_t *it, void **valuep) {
 	*valuep = it->cur->value;
 }
 
+void
+isc_ht_iter_currentkey(isc_ht_iter_t *it, unsigned char **key, size_t *keysize)
+{
+	REQUIRE(it != NULL);
+	REQUIRE(it->cur != NULL);
+	*key = it->cur->key;
+	*keysize = it->cur->keysize;
+}
+
 unsigned int
 isc_ht_count(isc_ht_t *ht) {
 	REQUIRE(ISC_HT_VALID(ht));
 
 	return(ht->count);
 }
-
-isc_result_t
-isc_ht_walk(isc_ht_t *ht, isc_ht_walkfn walkfn, void *udata) {
-	isc_uint32_t i;
-	isc_result_t res;
-
-	REQUIRE(ISC_HT_VALID(ht));
-
-	for (i = 0; i < ht->size; i++) {
-		isc_ht_node_t *cur = ht->table[i];
-		isc_ht_node_t *prev = NULL;
-		while (cur != NULL) {
-			if (walkfn == NULL) {
-				continue;
-			}
-			res = walkfn(udata, cur->key, cur->keysize, cur->value);
-			if (res != ISC_R_SUCCESS && res != ISC_R_EXISTS) {
-				return (res);
-			}
-			if (res == ISC_R_EXISTS) { /* remove this node */
-				isc_ht_node_t *tmp = cur;
-				cur = cur->next;
-				if (prev == NULL) {
-					ht->table[i] = cur;
-				} else {
-					prev->next = cur;
-				}
-				isc_mem_put(ht->mctx, tmp,
-					sizeof(isc_ht_node_t) + tmp->keysize);
-				ht->count--;
-			} else {
-				prev = cur;
-				cur = cur->next;
-			}
-		}
-	}
-
-	return (ISC_R_SUCCESS);
-}

lib/isc/include/isc/ht.h

@@ -94,10 +94,6 @@ isc_ht_find(const isc_ht_t *ht, const unsigned char *key,
 isc_result_t
 isc_ht_delete(isc_ht_t *ht, const unsigned char *key, isc_uint32_t keysize);
 
-
-typedef isc_result_t (*isc_ht_walkfn)(void *udata, const unsigned char *key,
-		isc_uint32_t keysize, void *data);
-
 /*%
  * Create an iterator for the hashtable; point '*itp' to it.
  */
@@ -130,6 +126,17 @@ isc_ht_iter_first(isc_ht_iter_t *it);
 isc_result_t
 isc_ht_iter_next(isc_ht_iter_t *it);
 
+/*%
+ * Delete current entry and set an iterator to the next entry.
+ *
+ * Returns:
+ * \li 	#ISC_R_SUCCESS	-- success
+ * \li	#ISC_R_NOMORE	-- end of hashtable reached
+ */
+isc_result_t
+isc_ht_iter_delcurrent_next(isc_ht_iter_t *it);
+
+
 /*%
  * Set 'value' to the current value under the iterator
  */
@@ -137,24 +144,11 @@ void
 isc_ht_iter_current(isc_ht_iter_t *it, void **valuep);
 
 /*%
- * Walks the hashtable, calling 'walkfn' on each node
+ * Set 'key' and 'keysize to the current key and keysize for the value
- *
+ * under the iterator
- * \li If 'walkfn' returns ISC_R_SUCCESS, walk is continued
- * \li If 'walkfn' returns ISC_R_EXISTS, walk is continued but the
- *     node is removed
- * \li If 'walkfn' returns anything else, walk is aborted and function returns
- *     this value
- *
- * Requires:
- * \li	'ht' is a valid hashtable
- * \li  'walkfn' is not NULL
- *
- * Returns:
- * \li	#ISC_R_SUCCESS		-- all is well
- * \li	Any other, as returned by 'walkfn'
  */
-isc_result_t
+void
-isc_ht_walk(isc_ht_t *ht, isc_ht_walkfn walkfn, void *udata);
+isc_ht_iter_currentkey(isc_ht_iter_t *it, unsigned char **key, size_t *keysize);
 
 /*%
  * Returns the number of items in the hashtable.

lib/isc/tests/ht_test.c

@@ -164,57 +164,18 @@ static void test_ht_full(int bits, int count) {
 	ATF_REQUIRE_EQ(ht, NULL);
 }
 
-static isc_uint32_t walked = 0;
+static void test_ht_iterator() {
-
-typedef enum {
-	REGULAR,
-	ERASEEVEN,
-	ERASEODD,
-	CRASH
-} walkmode_t;
-
-static isc_result_t walker(void *udata, const unsigned char *key,
-			   isc_uint32_t keysize, void *data)
-{
-	char mykey[16];
-	isc_uint64_t ii = (isc_uint64_t) data;
-	walkmode_t mode = (isc_uint64_t) udata;
-	ATF_REQUIRE_EQ(keysize, 16);
-
-	snprintf(mykey, 16, "%lld key of a raw hashtable!!", ii);
-	ATF_REQUIRE_EQ(memcmp(mykey, key, 16), 0);
-
-	walked++;
-	switch (mode) {
-	case REGULAR:
-		break;
-	case ERASEEVEN:
-		if (ii % 2 == 0) {
-			return (ISC_R_EXISTS);
-		}
-		break;
-	case ERASEODD:
-		if (ii % 2 != 0) {
-			return (ISC_R_EXISTS);
-		}
-		break;
-	case CRASH:
-		if (walked == 100) {
-			/* something as odd as possible */
-			return (ISC_R_HOSTUNREACH);
-		}
-		break;
-	}
-
-	return (ISC_R_SUCCESS);
-}
-
-static void test_ht_walk() {
 	isc_ht_t *ht = NULL;
 	isc_result_t result;
 	isc_mem_t *mctx = NULL;
+	isc_ht_iter_t * iter = NULL;
 	isc_int64_t i;
+	isc_int64_t v;
 	isc_uint32_t count = 10000;
+	isc_uint32_t walked;
+	unsigned char key[16];
+	unsigned char *tkey;
+	size_t tksize;
 
 	result = isc_mem_createx2(0, 0, default_memalloc, default_memfree,
 				  NULL, &mctx, 0);
@@ -228,35 +189,76 @@ static void test_ht_walk() {
 		 * Note that the string we're snprintfing is always > 16 bytes
 		 * so we are always filling the key.
 		 */
-		unsigned char key[16];
 		snprintf((char *)key, 16, "%lld key of a raw hashtable!!", i);
 		result = isc_ht_add(ht, key, 16, (void *) i);
 		ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
 	}
 
 	walked = 0;
-	result = isc_ht_walk(ht, walker, (void *) REGULAR);
+	result = isc_ht_iter_create(ht, &iter);
 	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+
+	for (result = isc_ht_iter_first(iter);
+	     result == ISC_R_SUCCESS;
+	     result = isc_ht_iter_next(iter))
+	{
+		isc_ht_iter_current(iter, (void**) &v);
+		isc_ht_iter_currentkey(iter, &tkey, &tksize);
+		ATF_REQUIRE_EQ(tksize, 16);
+		snprintf((char *)key, 16, "%lld key of a raw hashtable!!", v);
+		ATF_REQUIRE_EQ(memcmp(key, tkey, 16), 0);
+		walked++;
+	}
+	ATF_REQUIRE_EQ(walked, count);
+	ATF_REQUIRE_EQ(result, ISC_R_NOMORE);
+
+	/* erase odd */
+	walked = 0;
+	result = isc_ht_iter_first(iter);
+	while (result == ISC_R_SUCCESS) {
+		isc_ht_iter_current(iter, (void**) &v);
+		isc_ht_iter_currentkey(iter, &tkey, &tksize);
+		ATF_REQUIRE_EQ(tksize, 16);
+		snprintf((char *)key, 16, "%lld key of a raw hashtable!!", v);
+		ATF_REQUIRE_EQ(memcmp(key, tkey, 16), 0);
+		if (v % 2 == 0) {
+			result = isc_ht_iter_delcurrent_next(iter);
+		} else {
+			result = isc_ht_iter_next(iter);
+		}
+		walked++;
+	}
+	ATF_REQUIRE_EQ(result, ISC_R_NOMORE);
 	ATF_REQUIRE_EQ(walked, count);
 
+	/* erase even */
 	walked = 0;
-	result = isc_ht_walk(ht, walker, (void *) CRASH);
+	result = isc_ht_iter_first(iter);
-	ATF_REQUIRE_EQ(result, ISC_R_HOSTUNREACH);
+	while (result == ISC_R_SUCCESS) {
-	ATF_REQUIRE_EQ(walked, 100);
+		isc_ht_iter_current(iter, (void**) &v);
-
+		isc_ht_iter_currentkey(iter, &tkey, &tksize);
-	walked = 0;
+		ATF_REQUIRE_EQ(tksize, 16);
-	result = isc_ht_walk(ht, walker, (void *) ERASEODD);
+		snprintf((char *)key, 16, "%lld key of a raw hashtable!!", v);
-	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+		ATF_REQUIRE_EQ(memcmp(key, tkey, 16), 0);
-	ATF_REQUIRE_EQ(walked, count);
+		if (v % 2 == 1) {
-
+			result = isc_ht_iter_delcurrent_next(iter);
-	walked = 0;
+		} else {
-	result = isc_ht_walk(ht, walker, (void *) ERASEEVEN);
+			result = isc_ht_iter_next(iter);
-	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+		}
+		walked++;
+	}
+	ATF_REQUIRE_EQ(result, ISC_R_NOMORE);
 	ATF_REQUIRE_EQ(walked, count/2);
 
 	walked = 0;
-	result = isc_ht_walk(ht, walker, (void *) REGULAR);
+	for (result = isc_ht_iter_first(iter);
-	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+	     result == ISC_R_SUCCESS;
+	     result = isc_ht_iter_next(iter))
+	{
+		walked++;
+	}
+
+	ATF_REQUIRE_EQ(result, ISC_R_NOMORE);
 	ATF_REQUIRE_EQ(walked, 0);
 
 	isc_ht_destroy(&ht);
@@ -304,14 +306,14 @@ ATF_TC_BODY(isc_ht_32, tc) {
 	test_ht_full(32, 10000);
 }
 
-ATF_TC(isc_ht_walk);
+ATF_TC(isc_ht_iterator);
-ATF_TC_HEAD(isc_ht_walk, tc) {
+ATF_TC_HEAD(isc_ht_iterator, tc) {
-	atf_tc_set_md_var(tc, "descr", "hashtable walking");
+	atf_tc_set_md_var(tc, "descr", "hashtable iterator");
 }
 
-ATF_TC_BODY(isc_ht_walk, tc) {
+ATF_TC_BODY(isc_ht_iterator, tc) {
 	UNUSED(tc);
-	test_ht_walk();
+	test_ht_iterator();
 }
 
 /*
@@ -322,7 +324,7 @@ ATF_TP_ADD_TCS(tp) {
 	ATF_TP_ADD_TC(tp, isc_ht_8);
 	ATF_TP_ADD_TC(tp, isc_ht_1);
 	ATF_TP_ADD_TC(tp, isc_ht_32);
-	ATF_TP_ADD_TC(tp, isc_ht_walk);
+	ATF_TP_ADD_TC(tp, isc_ht_iterator);
 	return (atf_no_error());
 }