From 1c1ab955ed97a6e98d079232573c7c3d638f70a1 Mon Sep 17 00:00:00 2001 From: Witold Krecicki Date: Tue, 31 May 2016 23:01:53 +0200 Subject: [PATCH] Catalog zones: use iterators instead of isc_ht_walk [RT #42529] --- .gitignore | 2 + lib/dns/catz.c | 375 +++++++++++++++++-------------------- lib/dns/include/dns/catz.h | 12 +- lib/isc/ht.c | 96 +++++----- lib/isc/include/isc/ht.h | 36 ++-- lib/isc/tests/ht_test.c | 138 +++++++------- 6 files changed, 322 insertions(+), 337 deletions(-) diff --git a/.gitignore b/.gitignore index 65540e2f90..12a7a6f51b 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,8 @@ autom4te.cache/ *.so *.a *.la +*.gcno +*.gcda *_test *-symtbl.c timestamp diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 9364ff1fc0..fbafe9d275 100644 --- a/lib/dns/catz.c +++ b/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); - /* newzonewalk always returns ISC_R_SUCCESS */ - RUNTIME_CHECK(result == ISC_R_SUCCESS); + result = isc_ht_iter_create(newzone->entries, &iter); + INSIST(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); - /* - * oldzonewalk always returns ISC_R_EXISTS, so walk should return - * ISC_R_SUCCESS - */ - RUNTIME_CHECK(result == ISC_R_SUCCESS); + result = isc_ht_iter_create(target->entries, &iter); + 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); + + 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; - /* - * 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); + result = isc_ht_iter_create(zone->entries, &iter); 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, - catzs); + 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_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; - dns_catz_zones_t *catzs = (dns_catz_zones_t *) udata; - dns_catz_zone_t *zone = (dns_catz_zone_t *) data; - - UNUSED(key); - UNUSED(keysize); + isc_ht_iter_t *iter = NULL; + dns_catz_zone_t *zone; REQUIRE(catzs != NULL); - REQUIRE(zone != NULL); - 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); - return (ISC_R_EXISTS); + 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); + zone->active = ISC_FALSE; } - - return (ISC_R_SUCCESS); + INSIST(result == ISC_R_NOMORE); + 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 diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h index 73bd25028e..6c9a29eb6f 100644 --- a/lib/dns/include/dns/catz.h +++ b/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_view_t *view, isc_taskmgr_t *taskmgr, void *udata); - isc_result_t (*modzone)(dns_catz_entry_t *entry, dns_catz_zone_t *origin, - 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); + dns_catz_zoneop_fn_t addzone; + dns_catz_zoneop_fn_t modzone; + dns_catz_zoneop_fn_t delzone; void * udata; }; diff --git a/lib/isc/ht.c b/lib/isc/ht.c index 962ee888a4..beb9f8a31f 100644 --- a/lib/isc/ht.c +++ b/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); -} diff --git a/lib/isc/include/isc/ht.h b/lib/isc/include/isc/ht.h index 88aaaab15e..c9532b6374 100644 --- a/lib/isc/include/isc/ht.h +++ b/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 - * - * \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' + * Set 'key' and 'keysize to the current key and keysize for the value + * under the iterator */ -isc_result_t -isc_ht_walk(isc_ht_t *ht, isc_ht_walkfn walkfn, void *udata); +void +isc_ht_iter_currentkey(isc_ht_iter_t *it, unsigned char **key, size_t *keysize); /*% * Returns the number of items in the hashtable. diff --git a/lib/isc/tests/ht_test.c b/lib/isc/tests/ht_test.c index b8429b6282..60988e689d 100644 --- a/lib/isc/tests/ht_test.c +++ b/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; - -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() { +static void test_ht_iterator() { 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); - ATF_REQUIRE_EQ(result, ISC_R_HOSTUNREACH); - ATF_REQUIRE_EQ(walked, 100); - - walked = 0; - result = isc_ht_walk(ht, walker, (void *) ERASEODD); - ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); - ATF_REQUIRE_EQ(walked, count); - - walked = 0; - result = isc_ht_walk(ht, walker, (void *) ERASEEVEN); - ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + 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 == 1) { + 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/2); walked = 0; - result = isc_ht_walk(ht, walker, (void *) REGULAR); - ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + for (result = isc_ht_iter_first(iter); + 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_HEAD(isc_ht_walk, tc) { - atf_tc_set_md_var(tc, "descr", "hashtable walking"); +ATF_TC(isc_ht_iterator); +ATF_TC_HEAD(isc_ht_iterator, tc) { + 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()); }