From: Ondřej Surý <ondrej@sury.org>
Date: Sun, 22 Mar 2026 10:06:37 +0000 (+0100)
Subject: Make isc_radix_insert, dns_iptable_addprefix/merge return void
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=188aa43e48379572ffa758e45333c67214917d39;p=thirdparty%2Fbind9.git

Make isc_radix_insert, dns_iptable_addprefix/merge return void

isc_radix_insert can no longer fail: node allocation uses
isc_mem_get which aborts on OOM, and prefix copying was eliminated
by inlining. Propagate the void return through dns_iptable_addprefix,
dns_iptable_merge, dns_acl_any, dns_acl_none, and all their callers.
---

diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c
index 6a37a49f948..103f8688abd 100644
--- a/bin/named/controlconf.c
+++ b/bin/named/controlconf.c
@@ -964,7 +964,8 @@ update_listener(named_controls_t *cp, controllistener_t **listenerp,
 		result = cfg_acl_fromconfig(allow, config, aclctx,
 					    listener->mctx, 0, &new_acl);
 	} else {
-		result = dns_acl_any(listener->mctx, &new_acl);
+		dns_acl_any(listener->mctx, &new_acl);
+		result = ISC_R_SUCCESS;
 	}
 
 	if (control != NULL) {
@@ -1042,7 +1043,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp,
 			listener->readonly = cfg_obj_asboolean(readonly);
 		}
 	} else {
-		CHECK(dns_acl_any(mctx, &new_acl));
+		dns_acl_any(mctx, &new_acl);
 	}
 
 	dns_acl_attach(new_acl, &listener->acl);
diff --git a/bin/named/server.c b/bin/named/server.c
index be7ecd02251..86e0f961a50 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -3528,9 +3528,8 @@ cleanup:
 }
 #endif /* HAVE_DNSTAP */
 
-static isc_result_t
+static void
 create_mapped_acl(void) {
-	isc_result_t result;
 	dns_acl_t *acl = NULL;
 	struct in6_addr in6 = IN6ADDR_V4MAPPED_INIT;
 	isc_netaddr_t addr;
@@ -3538,13 +3537,8 @@ create_mapped_acl(void) {
 	isc_netaddr_fromin6(&addr, &in6);
 
 	dns_acl_create(isc_g_mctx, 1, &acl);
-
-	result = dns_iptable_addprefix(acl->iptable, &addr, 96, true);
-	if (result == ISC_R_SUCCESS) {
-		dns_acl_attach(acl, &named_g_mapped);
-	}
-	dns_acl_detach(&acl);
-	return result;
+	dns_iptable_addprefix(acl->iptable, &addr, 96, true);
+	named_g_mapped = acl;
 }
 
 isc_result_t
@@ -4033,7 +4027,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 							 mctx, 0, &excluded));
 			} else {
 				if (named_g_mapped == NULL) {
-					CHECK(create_mapped_acl());
+					create_mapped_acl();
 				}
 				dns_acl_attach(named_g_mapped, &excluded);
 			}
@@ -6410,7 +6404,7 @@ add_keydata_zone(dns_view_t *view, const char *directory, isc_mem_t *mctx) {
 
 	CHECK(dns_zonemgr_managezone(named_g_server->zonemgr, zone));
 
-	CHECK(dns_acl_none(mctx, &none));
+	dns_acl_none(mctx, &none);
 	dns_zone_setqueryacl(zone, none);
 	dns_zone_setqueryonacl(zone, none);
 	dns_acl_detach(&none);
diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c
index 2e02cde7ad5..f5a05e7e24b 100644
--- a/bin/named/statschannel.c
+++ b/bin/named/statschannel.c
@@ -3843,12 +3843,11 @@ add_listener(named_server_t *server, named_statschannel_t **listenerp,
 
 	allow = cfg_tuple_get(listen_params, "allow");
 	if (allow != NULL && cfg_obj_islist(allow)) {
-		result = cfg_acl_fromconfig(allow, config, aclctx,
-					    listener->mctx, 0, &new_acl);
+		CHECK(cfg_acl_fromconfig(allow, config, aclctx, listener->mctx,
+					 0, &new_acl));
 	} else {
-		result = dns_acl_any(listener->mctx, &new_acl);
+		dns_acl_any(listener->mctx, &new_acl);
 	}
-	CHECK(result);
 
 	dns_acl_attach(new_acl, &listener->acl);
 	dns_acl_detach(&new_acl);
@@ -3973,7 +3972,8 @@ update_listener(named_server_t *server, named_statschannel_t **listenerp,
 		result = cfg_acl_fromconfig(allow, config, aclctx,
 					    listener->mctx, 0, &new_acl);
 	} else {
-		result = dns_acl_any(listener->mctx, &new_acl);
+		dns_acl_any(listener->mctx, &new_acl);
+		result = ISC_R_SUCCESS;
 	}
 
 	if (result == ISC_R_SUCCESS) {
diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c
index 89846353f57..64be4515c2a 100644
--- a/bin/named/zoneconf.c
+++ b/bin/named/zoneconf.c
@@ -1862,8 +1862,8 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		obj = NULL;
 		(void)named_config_get(nooptions, "allow-transfer", &obj);
 		if (obj == NULL) {
-			dns_acl_t *none;
-			CHECK(dns_acl_none(mctx, &none));
+			dns_acl_t *none = NULL;
+			dns_acl_none(mctx, &none);
 			dns_zone_setxfracl(zone, none);
 			dns_acl_detach(&none);
 		}
diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c
index f722fdcb813..d7230cf65ed 100644
--- a/bin/plugins/filter-a.c
+++ b/bin/plugins/filter-a.c
@@ -284,7 +284,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters,
 					 (cfg_aclconfctx_t *)aclctx, mctx, 0,
 					 &inst->a_acl));
 	} else {
-		CHECK(dns_acl_any(mctx, &inst->a_acl));
+		dns_acl_any(mctx, &inst->a_acl);
 	}
 
 cleanup:
diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c
index 6a7a8d25e9b..b698ede7a15 100644
--- a/bin/plugins/filter-aaaa.c
+++ b/bin/plugins/filter-aaaa.c
@@ -287,7 +287,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters,
 					 (cfg_aclconfctx_t *)aclctx, mctx, 0,
 					 &inst->aaaa_acl));
 	} else {
-		CHECK(dns_acl_any(mctx, &inst->aaaa_acl));
+		dns_acl_any(mctx, &inst->aaaa_acl);
 	}
 
 cleanup:
diff --git a/bin/plugins/synthrecord.c b/bin/plugins/synthrecord.c
index 1825f584520..9001d8f3be3 100644
--- a/bin/plugins/synthrecord.c
+++ b/bin/plugins/synthrecord.c
@@ -512,7 +512,8 @@ synthrecord_parseallowsynth(synthrecord_t *inst, const cfg_obj_t *cfg,
 	result = cfg_map_get(synthrecordcfg, "allow-synth", &obj);
 
 	if (result == ISC_R_NOTFOUND) {
-		return dns_acl_any(inst->mctx, &inst->allowedsynth);
+		dns_acl_any(inst->mctx, &inst->allowedsynth);
+		return ISC_R_SUCCESS;
 	}
 
 	if (result != ISC_R_SUCCESS) {
diff --git a/bin/tests/system/dyndb/driver/zone.c b/bin/tests/system/dyndb/driver/zone.c
index 37724d44280..388aeae975c 100644
--- a/bin/tests/system/dyndb/driver/zone.c
+++ b/bin/tests/system/dyndb/driver/zone.c
@@ -84,12 +84,7 @@ create_zone(sample_instance_t *const inst, dns_name_t *const name,
 	}
 
 	/* This is completely insecure - use some sensible values instead! */
-	result = dns_acl_any(inst->mctx, &acl_any);
-	if (result != ISC_R_SUCCESS) {
-		log_write(ISC_LOG_ERROR, "create_zone: dns_acl_any -> %s\n",
-			  isc_result_totext(result));
-		goto cleanup;
-	}
+	dns_acl_any(inst->mctx, &acl_any);
 	dns_zone_setupdateacl(raw, acl_any);
 	dns_zone_setqueryacl(raw, acl_any);
 	dns_zone_setxfracl(raw, acl_any);
diff --git a/lib/dns/acl.c b/lib/dns/acl.c
index 03c5bb5212b..aa86643fbb0 100644
--- a/lib/dns/acl.c
+++ b/lib/dns/acl.c
@@ -61,37 +61,30 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) {
  * "any" is a positive iptable entry with bit length 0.
  * "none" is the same as "!any".
  */
-static isc_result_t
+static void
 dns_acl_anyornone(isc_mem_t *mctx, bool neg, dns_acl_t **target) {
-	isc_result_t result;
 	dns_acl_t *acl = NULL;
 
 	dns_acl_create(mctx, 0, &acl);
-
-	result = dns_iptable_addprefix(acl->iptable, NULL, 0, !neg);
-	if (result != ISC_R_SUCCESS) {
-		dns_acl_detach(&acl);
-		return result;
-	}
+	dns_iptable_addprefix(acl->iptable, NULL, 0, !neg);
 
 	*target = acl;
-	return result;
 }
 
 /*
  * Create a new ACL that matches everything.
  */
-isc_result_t
+void
 dns_acl_any(isc_mem_t *mctx, dns_acl_t **target) {
-	return dns_acl_anyornone(mctx, false, target);
+	dns_acl_anyornone(mctx, false, target);
 }
 
 /*
  * Create a new ACL that matches nothing.
  */
-isc_result_t
+void
 dns_acl_none(isc_mem_t *mctx, dns_acl_t **target) {
-	return dns_acl_anyornone(mctx, true, target);
+	dns_acl_anyornone(mctx, true, target);
 }
 
 /*
@@ -342,7 +335,7 @@ dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, bool pos) {
 	 * node_count value is set correctly afterward.
 	 */
 	nodes = max_node + dns_acl_node_count(dest);
-	RETERR(dns_iptable_merge(dest->iptable, source->iptable, pos));
+	dns_iptable_merge(dest->iptable, source->iptable, pos);
 	if (nodes > dns_acl_node_count(dest)) {
 		dns_acl_node_count(dest) = nodes;
 	}
diff --git a/lib/dns/include/dns/acl.h b/lib/dns/include/dns/acl.h
index bf11af019f1..4796c43777a 100644
--- a/lib/dns/include/dns/acl.h
+++ b/lib/dns/include/dns/acl.h
@@ -125,13 +125,13 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target);
  * length is 0.
  */
 
-isc_result_t
+void
 dns_acl_any(isc_mem_t *mctx, dns_acl_t **target);
 /*%<
  * Create a new ACL that matches everything.
  */
 
-isc_result_t
+void
 dns_acl_none(isc_mem_t *mctx, dns_acl_t **target);
 /*%<
  * Create a new ACL that matches nothing.
diff --git a/lib/dns/include/dns/iptable.h b/lib/dns/include/dns/iptable.h
index 939e8e72324..807e1691c68 100644
--- a/lib/dns/include/dns/iptable.h
+++ b/lib/dns/include/dns/iptable.h
@@ -45,14 +45,14 @@ dns_iptable_create(isc_mem_t *mctx, dns_iptable_t **target);
  * Create a new IP table and the underlying radix structure
  */
 
-isc_result_t
+void
 dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr,
 		      uint16_t bitlen, bool pos);
 /*
  * Add an IP prefix to an existing IP table
  */
 
-isc_result_t
+void
 dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos);
 /*
  * Merge one IP table into another one.
diff --git a/lib/dns/iptable.c b/lib/dns/iptable.c
index 5acd6d05843..a2cf2754809 100644
--- a/lib/dns/iptable.c
+++ b/lib/dns/iptable.c
@@ -43,30 +43,22 @@ static bool dns_iptable_pos = true;
 /*
  * Add an IP prefix to an existing IP table
  */
-static isc_result_t
+static void
 iptable_addentry(dns_iptable_t *tab, isc_prefix_t *pfx, bool pos) {
 	isc_radix_node_t *node = NULL;
-	isc_result_t result;
 
-	result = isc_radix_insert(tab->radix, &node, NULL, pfx);
-	if (result != ISC_R_SUCCESS) {
-		return result;
-	}
+	isc_radix_insert(tab->radix, &node, NULL, pfx);
 
 	/* Preserve first-match semantics: don't overwrite existing data */
 	int fam = ISC_RADIX_FAMILY(pfx);
 	if (node->data[fam] == NULL) {
 		node->data[fam] = pos ? &dns_iptable_pos : &dns_iptable_neg;
 	}
-
-	return ISC_R_SUCCESS;
 }
 
-isc_result_t
+void
 dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr,
 		      uint16_t bitlen, bool pos) {
-	isc_result_t result;
-
 	INSIST(DNS_IPTABLE_VALID(tab));
 	INSIST(tab->radix != NULL);
 
@@ -78,29 +70,27 @@ dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr,
 		isc_prefix_t pfx4 = { .family = AF_INET, .bitlen = 0 };
 		isc_prefix_t pfx6 = { .family = AF_INET6, .bitlen = 0 };
 
-		result = iptable_addentry(tab, &pfx4, pos);
-		if (result != ISC_R_SUCCESS) {
-			return result;
-		}
-		return iptable_addentry(tab, &pfx6, pos);
+		iptable_addentry(tab, &pfx4, pos);
+		iptable_addentry(tab, &pfx6, pos);
+		return;
 	}
 
 	isc_prefix_t pfx;
 	NETADDR_TO_PREFIX_T(addr, pfx, bitlen);
-	return iptable_addentry(tab, &pfx, pos);
+	iptable_addentry(tab, &pfx, pos);
 }
 
 /*
  * Merge one IP table into another one.
  */
-isc_result_t
+void
 dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos) {
 	isc_radix_node_t *node, *new_node;
-	int i, max_node = 0;
+	int max_node = 0;
 
 	RADIX_WALK(source->radix->head, node) {
 		new_node = NULL;
-		RETERR(isc_radix_insert(tab->radix, &new_node, node, NULL));
+		isc_radix_insert(tab->radix, &new_node, node, NULL);
 
 		/*
 		 * If we're negating a nested ACL, then we should
@@ -110,9 +100,11 @@ dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos) {
 		 * could be a security risk.  To prevent this, we
 		 * just leave the negative nodes negative.
 		 */
-		for (i = 0; i < RADIX_FAMILIES; i++) {
+		for (int i = 0; i < RADIX_FAMILIES; i++) {
 			if (!pos) {
-				if (node->data[i] && *(bool *)node->data[i]) {
+				if (node->data[i] != NULL &&
+				    *(bool *)node->data[i])
+				{
 					new_node->data[i] = &dns_iptable_neg;
 				}
 			}
@@ -124,7 +116,6 @@ dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos) {
 	RADIX_WALK_END;
 
 	tab->radix->num_added_node += max_node;
-	return ISC_R_SUCCESS;
 }
 
 static void
diff --git a/lib/isc/include/isc/radix.h b/lib/isc/include/isc/radix.h
index b70160124ff..c5b48532872 100644
--- a/lib/isc/include/isc/radix.h
+++ b/lib/isc/include/isc/radix.h
@@ -115,7 +115,7 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target,
  * \li	ISC_R_SUCCESS
  */
 
-isc_result_t
+void
 isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 		 isc_radix_node_t *source, isc_prefix_t *prefix);
 /*%<
@@ -127,9 +127,6 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
  * \li	'target' is not NULL and "*target" is NULL.
  * \li	'prefix' to be valid or 'source' to be non NULL and contain
  *	a valid prefix.
- *
- * Returns:
- * \li	ISC_R_SUCCESS
  */
 
 void
diff --git a/lib/isc/radix.c b/lib/isc/radix.c
index 1863ddc0708..0e1139f5375 100644
--- a/lib/isc/radix.c
+++ b/lib/isc/radix.c
@@ -11,12 +11,6 @@
  * information regarding copyright ownership.
  */
 
-/*
- * This source was adapted from MRT's RCS Ids:
- * Id: radix.c,v 1.10.2.1 1999/11/29 05:16:24 masaki Exp
- * Id: prefix.c,v 1.37.2.9 2000/03/10 02:53:19 labovit Exp
- */
-
 #include <inttypes.h>
 
 #include <isc/mem.h>
@@ -209,7 +203,7 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target,
 	return ISC_R_SUCCESS;
 }
 
-isc_result_t
+void
 isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 		 isc_radix_node_t *source, isc_prefix_t *prefix) {
 	isc_radix_node_t *node;
@@ -254,7 +248,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 		radix->head = node;
 		radix->num_active_node++;
 		*target = node;
-		return ISC_R_SUCCESS;
+		return;
 	}
 
 	u_char *addr = isc_prefix_touchar(prefix);
@@ -334,7 +328,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 				}
 			}
 			*target = node;
-			return ISC_R_SUCCESS;
+			return;
 		} else {
 			node->prefix = *prefix;
 		}
@@ -357,7 +351,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 			node->node_num[ISC_RADIX_FAMILY(prefix)] = next;
 		}
 		*target = node;
-		return ISC_R_SUCCESS;
+		return;
 	}
 
 	isc_radix_node_t *new_node = radix_node_create(radix->mctx, prefix,
@@ -396,7 +390,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 			node->left = new_node;
 		}
 		*target = new_node;
-		return ISC_R_SUCCESS;
+		return;
 	}
 
 	if (bitlen == differ_bit) {
@@ -446,7 +440,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
 	}
 
 	*target = new_node;
-	return ISC_R_SUCCESS;
+	return;
 }
 
 void
diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c
index 9d31fdf07d4..af98ebbca69 100644
--- a/lib/isccfg/aclconf.c
+++ b/lib/isccfg/aclconf.c
@@ -744,8 +744,7 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx,
 			 * the nestedacl element, not the iptable entry.
 			 */
 			setpos = (nest_level != 0 || !neg);
-			CHECK(dns_iptable_addprefix(iptab, &addr, bitlen,
-						    setpos));
+			dns_iptable_addprefix(iptab, &addr, bitlen, setpos);
 
 			if (nest_level > 0) {
 				INSIST(dacl->length < dacl->alloc);
@@ -814,8 +813,7 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx,
 			if (strcasecmp(name, "any") == 0) {
 				/* Iptable entry with zero bit length. */
 				setpos = (nest_level != 0 || !neg);
-				CHECK(dns_iptable_addprefix(iptab, NULL, 0,
-							    setpos));
+				dns_iptable_addprefix(iptab, NULL, 0, setpos);
 
 				if (nest_level != 0) {
 					INSIST(dacl->length < dacl->alloc);
@@ -833,8 +831,7 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx,
 				 * "!none;".
 				 */
 				setpos = (nest_level != 0 || neg);
-				CHECK(dns_iptable_addprefix(iptab, NULL, 0,
-							    setpos));
+				dns_iptable_addprefix(iptab, NULL, 0, setpos);
 
 				if (!neg) {
 					dacl->has_negatives = !neg;
diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c
index 71ffda00460..da05573cefc 100644
--- a/lib/ns/interfacemgr.c
+++ b/lib/ns/interfacemgr.c
@@ -840,8 +840,7 @@ setup_locals(isc_interface_t *interface, dns_acl_t *localhost,
 
 	/* First add localhost address */
 	prefixlen = (netaddr->family == AF_INET) ? 32 : 128;
-	RETERR(dns_iptable_addprefix(localhost->iptable, netaddr, prefixlen,
-				     true));
+	dns_iptable_addprefix(localhost->iptable, netaddr, prefixlen, true);
 
 	/* Then add localnets prefix */
 	result = isc_netaddr_masktoprefixlen(&interface->netmask, &prefixlen);
@@ -870,8 +869,7 @@ setup_locals(isc_interface_t *interface, dns_acl_t *localhost,
 		return ISC_R_SUCCESS;
 	}
 
-	RETERR(dns_iptable_addprefix(localnets->iptable, netaddr, prefixlen,
-				     true));
+	dns_iptable_addprefix(localnets->iptable, netaddr, prefixlen, true);
 
 	return ISC_R_SUCCESS;
 }
diff --git a/tests/dns/acl_test.c b/tests/dns/acl_test.c
index 5861a15e8f4..a910b3d3442 100644
--- a/tests/dns/acl_test.c
+++ b/tests/dns/acl_test.c
@@ -52,11 +52,9 @@ ISC_RUN_TEST_IMPL(dns_acl_isinsecure) {
 
 	UNUSED(state);
 
-	result = dns_acl_any(isc_g_mctx, &any);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	dns_acl_any(isc_g_mctx, &any);
 
-	result = dns_acl_none(isc_g_mctx, &none);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	dns_acl_none(isc_g_mctx, &none);
 
 	dns_acl_create(isc_g_mctx, 1, &notnone);
 
diff --git a/tests/isc/radix_test.c b/tests/isc/radix_test.c
index 7b916aa5454..8a4599f64ba 100644
--- a/tests/isc/radix_test.c
+++ b/tests/isc/radix_test.c
@@ -49,8 +49,7 @@ insert_prefix(isc_radix_tree_t *radix, const char *str, uint16_t bitlen,
 
 	prefix_from_str(str, bitlen, &pfx);
 
-	isc_result_t result = isc_radix_insert(radix, &node, NULL, &pfx);
-	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_radix_insert(radix, &node, NULL, &pfx);
 	node->data[RADIX_V4] = data;
 
 	return node;