From 3bb47bc6cd83b8451cf86a2b4ab5127183dbb42c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Mon, 24 Feb 2025 13:58:13 +0100
Subject: [PATCH] Remove MAKE_EMPTY() macro from dns_name unit

The MAKE_EMPTY() macro was clearing up the output variable in case of
the failure.  However, this was breaking the usual design pattern that
the output variables are left in indeterminate state or we don't touch
them at all when a failure occurs.  Remove the macro and change the
dns_name_downcase() to not touch the name contents until success.
---
 lib/dns/name.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/lib/dns/name.c b/lib/dns/name.c
index 80f8b54154..f05f8a97de 100644
--- a/lib/dns/name.c
+++ b/lib/dns/name.c
@@ -46,17 +46,6 @@ typedef enum {
 	ft_at
 } ft_state;
 
-/*%
- * Note:  If additional attributes are added that should not be set for
- *	  empty names, MAKE_EMPTY() must be changed so it clears them.
- */
-#define MAKE_EMPTY(name)                           \
-	{                                          \
-		name->ndata = NULL;                \
-		name->length = 0;                  \
-		name->attributes.absolute = false; \
-	}
-
 /*%
  * Note that the name data must be a char array, not a string
  * literal, to avoid compiler warnings about discarding
@@ -743,11 +732,6 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source,
 
 	REQUIRE(DNS_NAME_BINDABLE(name));
 
-	/*
-	 * Make 'name' empty in case of failure.
-	 */
-	MAKE_EMPTY(name);
-
 	/*
 	 * Set up the state machine.
 	 */
@@ -1256,6 +1240,7 @@ dns_name_downcase(const dns_name_t *source, dns_name_t *name,
 
 	REQUIRE(DNS_NAME_VALID(source));
 	REQUIRE(DNS_NAME_VALID(name));
+
 	if (source == name) {
 		REQUIRE(!name->attributes.readonly);
 		isc_buffer_init(&buffer, source->ndata, source->length);
@@ -1266,18 +1251,18 @@ dns_name_downcase(const dns_name_t *source, dns_name_t *name,
 		REQUIRE((target != NULL && ISC_BUFFER_VALID(target)) ||
 			(target == NULL && ISC_BUFFER_VALID(name->buffer)));
 		if (target == NULL) {
+			if (source->length > name->buffer->length) {
+				return ISC_R_NOSPACE;
+			}
 			target = name->buffer;
 			isc_buffer_clear(name->buffer);
+		} else if (source->length > target->length - target->used) {
+			return ISC_R_NOSPACE;
 		}
 		ndata = (unsigned char *)target->base + target->used;
 		name->ndata = ndata;
 	}
 
-	if (source->length > (target->length - target->used)) {
-		MAKE_EMPTY(name);
-		return ISC_R_NOSPACE;
-	}
-
 	/* label lengths are < 64 so tolower() does not affect them */
 	isc_ascii_lowercopy(ndata, source->ndata, source->length);
 
@@ -1400,7 +1385,6 @@ dns_name_fromwire(dns_name_t *const name, isc_buffer_t *const source,
 	const uint32_t name_max = ISC_MIN(DNS_NAME_MAXWIRE,
 					  isc_buffer_availablelength(target));
 	uint32_t name_len = 0;
-	MAKE_EMPTY(name); /* in case of failure */
 
 	/*
 	 * After chasing a compression pointer, these variables refer to the
@@ -1628,11 +1612,9 @@ dns_name_concatenate(const dns_name_t *prefix, const dns_name_t *suffix,
 		length += suffix->length;
 	}
 	if (length > DNS_NAME_MAXWIRE) {
-		MAKE_EMPTY(name);
 		return DNS_R_NAMETOOLONG;
 	}
 	if (length > nrem) {
-		MAKE_EMPTY(name);
 		return ISC_R_NOSPACE;
 	}
 
@@ -1672,11 +1654,6 @@ dns_name_dup(const dns_name_t *source, isc_mem_t *mctx, dns_name_t *target) {
 	REQUIRE(DNS_NAME_VALID(target));
 	REQUIRE(DNS_NAME_BINDABLE(target));
 
-	/*
-	 * Make 'target' empty in case of failure.
-	 */
-	MAKE_EMPTY(target);
-
 	target->ndata = isc_mem_get(mctx, source->length);
 
 	memmove(target->ndata, source->ndata, source->length);