From c70bb3deb54be52e1210e2c2e549fc568b6f356a Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Wed, 7 Dec 2022 19:58:40 +0000 Subject: [PATCH] Remove some remnants of bitstring labels * rbt node chains were sized to allow for bitstring labels, so they had 256 levels; but in the absence of bistrings, 128 is enough. * dns_byaddr_createptrname() had a redundant options argument, and a very outdated doc comment. * A number of comments referred to bitstring labels in a way that is no longer helpful. (A few informative comments remain.) --- CHANGES | 2 ++ bin/delv/delv.c | 3 +-- bin/dig/dighost.c | 3 +-- bin/tests/system/dyndb/driver/syncptr.c | 2 +- bin/tools/mdig.c | 5 ++--- lib/dns/byaddr.c | 4 +--- lib/dns/include/dns/byaddr.h | 9 ++------- lib/dns/include/dns/rbt.h | 23 +++++++++-------------- lib/dns/rbt.c | 8 +++----- lib/dns/result.c | 2 -- 10 files changed, 22 insertions(+), 39 deletions(-) diff --git a/CHANGES b/CHANGES index 6f5472d783..b8ec767995 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +6086. [cleanup] Remove some remnants of bitstring labels. [GL !7196] + 6085. [func] Add isc_time_monotonic() to simplify time measurements. [GL !7468] diff --git a/bin/delv/delv.c b/bin/delv/delv.c index 132d5dda34..13c85b240a 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -1688,10 +1688,9 @@ get_reverse(char *reverse, size_t len, char *value, bool strict) { /* This is a valid IPv6 address. */ dns_fixedname_t fname; dns_name_t *name; - unsigned int options = 0; name = dns_fixedname_initname(&fname); - result = dns_byaddr_createptrname(&addr, options, name); + result = dns_byaddr_createptrname(&addr, name); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 1a1c14feca..6e4aefc51e 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -328,10 +328,9 @@ get_reverse(char *reverse, size_t len, char *value, bool strict) { /* This is a valid IPv6 address. */ dns_fixedname_t fname; dns_name_t *name; - unsigned int options = 0; name = dns_fixedname_initname(&fname); - result = dns_byaddr_createptrname(&addr, options, name); + result = dns_byaddr_createptrname(&addr, name); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/bin/tests/system/dyndb/driver/syncptr.c b/bin/tests/system/dyndb/driver/syncptr.c index 5124df32b4..edf6f8271a 100644 --- a/bin/tests/system/dyndb/driver/syncptr.c +++ b/bin/tests/system/dyndb/driver/syncptr.c @@ -167,7 +167,7 @@ syncptr_find_zone(sample_instance_t *inst, dns_rdata_t *rdata, dns_name_t *name, * @example * 192.168.0.1 -> 1.0.168.192.in-addr.arpa */ - result = dns_byaddr_createptrname(&isc_ip, 0, name); + result = dns_byaddr_createptrname(&isc_ip, name); if (result != ISC_R_SUCCESS) { log_write(ISC_LOG_ERROR, "syncptr_find_zone: dns_byaddr_createptrname -> %s\n", diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index d0bed78d07..b964dabdec 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -1091,11 +1091,10 @@ get_reverse(char *reverse, size_t len, const char *value) { /* This is a valid IPv6 address. */ dns_fixedname_t fname; dns_name_t *name; - unsigned int options = 0; name = dns_fixedname_initname(&fname); - result = dns_byaddr_createptrname(&addr, options, name); - CHECK("dns_byaddr_createptrname2", result); + result = dns_byaddr_createptrname(&addr, name); + CHECK("dns_byaddr_createptrname", result); dns_name_format(name, reverse, (unsigned int)len); return; } else { diff --git a/lib/dns/byaddr.c b/lib/dns/byaddr.c index 1b17b24934..6297230ceb 100644 --- a/lib/dns/byaddr.c +++ b/lib/dns/byaddr.c @@ -40,8 +40,7 @@ static char hex_digits[] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; isc_result_t -dns_byaddr_createptrname(const isc_netaddr_t *address, unsigned int options, - dns_name_t *name) { +dns_byaddr_createptrname(const isc_netaddr_t *address, dns_name_t *name) { char textname[128]; const unsigned char *bytes; int i; @@ -50,7 +49,6 @@ dns_byaddr_createptrname(const isc_netaddr_t *address, unsigned int options, unsigned int len; REQUIRE(address != NULL); - UNUSED(options); /* * We create the text representation and then convert to a diff --git a/lib/dns/include/dns/byaddr.h b/lib/dns/include/dns/byaddr.h index 20e0f53dd9..4c18ddad28 100644 --- a/lib/dns/include/dns/byaddr.h +++ b/lib/dns/include/dns/byaddr.h @@ -48,14 +48,9 @@ ISC_LANG_BEGINDECLS isc_result_t -dns_byaddr_createptrname(const isc_netaddr_t *address, unsigned int options, - dns_name_t *name); +dns_byaddr_createptrname(const isc_netaddr_t *address, dns_name_t *name); /*%< - * Creates a name that would be used in a PTR query for this address. The - * nibble flag indicates that the 'nibble' format is to be used if an IPv6 - * address is provided, instead of the 'bitstring' format. Since we dropped - * the support of the bitstring labels, it is expected that the flag is always - * set. 'options' are the same as for dns_byaddr_create(). + * Creates a name that would be used in a PTR query for this address. * * Requires: * diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h index 4651edcf4d..bf5b23eb67 100644 --- a/lib/dns/include/dns/rbt.h +++ b/lib/dns/include/dns/rbt.h @@ -211,15 +211,13 @@ typedef void (*dns_rbtdeleter_t)(void *, void *); */ /*% - * The number of level blocks to allocate at a time. Currently the maximum - * number of levels is allocated directly in the structure, but future - * revisions of this code might have a static initial block with dynamic - * growth. Allocating space for 256 levels when the tree is almost never that - * deep is wasteful, but it's not clear that it matters, since the waste is - * only 2MB for 1000 concurrently active chains on a system with 64-bit - * pointers. + * The number of level blocks to allocate at a time, same as the maximum + * number of labels. Allocating space for 128 levels when the tree is + * almost never that deep is wasteful, but it's not clear that it matters, + * since the waste is only 1MB for 1000 concurrently active chains on a + * system with 64-bit pointers. */ -#define DNS_RBT_LEVELBLOCK 254 +#define DNS_RBT_LEVELBLOCK 127 typedef struct dns_rbtnodechain { unsigned int magic; @@ -231,12 +229,9 @@ typedef struct dns_rbtnodechain { */ dns_rbtnode_t *end; /*% - * The maximum number of labels in a name is 128; bitstrings mean - * a conceptually very large number (which I have not bothered to - * compute) of logical levels because splitting can potentially occur - * at each bit. However, DNSSEC restricts the number of "logical" - * labels in a name to 255, meaning only 254 pointers are needed - * in the worst case. + * Currently the maximum number of levels is allocated directly in + * the structure, but future revisions of this code might have a + * static initial block with dynamic growth. */ dns_rbtnode_t *levels[DNS_RBT_LEVELBLOCK]; /*% diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 2f51e9564d..eb3460c8ee 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -793,9 +793,8 @@ dns_rbt_findnode(dns_rbt_t *rbt, const dns_name_t *name, dns_name_t *foundname, /* * search_name is the name segment being sought in each tree level. * By using a fixedname, the search_name will definitely have offsets - * for use by any splitting. - * By using dns_name_clone, no name data should be copied thanks to - * the lack of bitstring labels. + * for use by any splitting. By using dns_name_clone, no name data + * should be copied. */ search_name = dns_fixedname_initname(&fixedsearchname); INSIST(search_name != NULL); @@ -949,8 +948,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, const dns_name_t *name, dns_name_t *foundname, /* * All of the labels have been tried against the hash - * table. Since we dropped the support of bitstring - * labels, the name isn't in the table. + * table. */ current = NULL; continue; diff --git a/lib/dns/result.c b/lib/dns/result.c index 65e2a4a454..86b7a8f6fb 100644 --- a/lib/dns/result.c +++ b/lib/dns/result.c @@ -44,14 +44,12 @@ dns_result_torcode(isc_result_t result) { case ISC_R_RANGE: case ISC_R_UNEXPECTEDEND: case DNS_R_BADAAAA: - /* case DNS_R_BADBITSTRING: deprecated */ case DNS_R_BADCKSUM: case DNS_R_BADCLASS: case DNS_R_BADLABELTYPE: case DNS_R_BADPOINTER: case DNS_R_BADTTL: case DNS_R_BADZONE: - /* case DNS_R_BITSTRINGTOOLONG: deprecated */ case DNS_R_EXTRADATA: case DNS_R_LABELTOOLONG: case DNS_R_NOREDATA: