Cleanup validator code

Detach the subvalidator immediately after saving the name pointing
into the caller's message.  Move detaching val->parent to
destroy_validator.

Store the validator result instead of eresult. The code only needs
a single variable.

lib/dns/validator.c

@@ -398,8 +398,7 @@ fetch_callback_dnskey(void *arg) {
 	dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
 	dns_validator_t *val = resp->arg;
 	dns_rdataset_t *rdataset = &val->frdataset;
-	isc_result_t eresult = resp->result;
-	isc_result_t result;
+	isc_result_t result = resp->result;
 
 	/* Free resources which are not of interest. */
 	if (resp->node != NULL) {
@@ -421,7 +420,7 @@ fetch_callback_dnskey(void *arg) {
 		goto cleanup;
 	}
 
-	switch (eresult) {
+	switch (result) {
 	case ISC_R_SUCCESS:
 	case DNS_R_NCACHENXRRSET:
 		/*
@@ -429,13 +428,13 @@ fetch_callback_dnskey(void *arg) {
 		 * RRset or a NODATA response.
 		 */
 		validator_log(val, ISC_LOG_DEBUG(3), "%s with trust %s",
-			      eresult == ISC_R_SUCCESS ? "keyset"
-						       : "NCACHENXRRSET",
+			      result == ISC_R_SUCCESS ? "keyset"
+						      : "NCACHENXRRSET",
 			      dns_trust_totext(rdataset->trust));
 		/*
 		 * Only extract the dst key if the keyset exists and is secure.
 		 */
-		if (eresult == ISC_R_SUCCESS &&
+		if (result == ISC_R_SUCCESS &&
 		    rdataset->trust >= dns_trust_secure)
 		{
 			result = validate_helper_run(val,
@@ -447,7 +446,7 @@ fetch_callback_dnskey(void *arg) {
 	default:
 		validator_log(val, ISC_LOG_DEBUG(3),
 			      "fetch_callback_dnskey: got %s",
-			      isc_result_totext(eresult));
+			      isc_result_totext(result));
 		result = DNS_R_BROKENCHAIN;
 	}
 
@@ -466,8 +465,7 @@ fetch_callback_ds(void *arg) {
 	dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg;
 	dns_validator_t *val = resp->arg;
 	dns_rdataset_t *rdataset = &val->frdataset;
-	isc_result_t eresult = resp->result;
-	isc_result_t result;
+	isc_result_t result = resp->result;
 	bool trustchain;
 
 	/*
@@ -497,7 +495,7 @@ fetch_callback_ds(void *arg) {
 	}
 
 	if (trustchain) {
-		switch (eresult) {
+		switch (result) {
 		case ISC_R_SUCCESS:
 			/*
 			 * We looked for a DS record as part of
@@ -520,18 +518,18 @@ fetch_callback_ds(void *arg) {
 			 */
 			validator_log(val, ISC_LOG_DEBUG(3),
 				      "falling back to insecurity proof (%s)",
-				      isc_result_totext(eresult));
+				      isc_result_totext(result));
 			result = proveunsecure(val, false, false);
 			break;
 		default:
 			validator_log(val, ISC_LOG_DEBUG(3),
 				      "fetch_callback_ds: got %s",
-				      isc_result_totext(eresult));
+				      isc_result_totext(result));
 			result = DNS_R_BROKENCHAIN;
 			break;
 		}
 	} else {
-		switch (eresult) {
+		switch (result) {
 		case DNS_R_NXDOMAIN:
 		case DNS_R_NCACHENXDOMAIN:
 			/*
@@ -554,7 +552,7 @@ fetch_callback_ds(void *arg) {
 		case DNS_R_NXRRSET:
 		case DNS_R_NCACHENXRRSET:
 			if (isdelegation(resp->foundname, &val->frdataset,
-					 eresult))
+					 result))
 			{
 				/*
 				 * Failed to find a DS while trying to prove
@@ -575,7 +573,7 @@ fetch_callback_ds(void *arg) {
 		default:
 			validator_log(val, ISC_LOG_DEBUG(3),
 				      "fetch_callback_ds: got %s",
-				      isc_result_totext(eresult));
+				      isc_result_totext(result));
 			result = DNS_R_BROKENCHAIN;
 		}
 	}
@@ -597,7 +595,8 @@ validator_callback_dnskey(void *arg) {
 	dns_validator_t *val = subvalidator->parent;
 	isc_result_t result = subvalidator->result;
 
-	val->subvalidator = NULL;
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&val->subvalidator);
 
 	if (CANCELED(val) || CANCELING(val)) {
 		result = ISC_R_CANCELED;
@@ -634,9 +633,6 @@ validator_callback_dnskey(void *arg) {
 	}
 
 cleanup:
-	dns_validator_detach(&subvalidator->parent);
-	dns_validator_shutdown(subvalidator);
-	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -651,7 +647,8 @@ validator_callback_ds(void *arg) {
 	dns_validator_t *val = subvalidator->parent;
 	isc_result_t result = subvalidator->result;
 
-	val->subvalidator = NULL;
+	dns_validator_shutdown(val->subvalidator);
+	dns_validator_detach(&val->subvalidator);
 
 	if (CANCELED(val) || CANCELING(val)) {
 		result = ISC_R_CANCELED;
@@ -696,9 +693,6 @@ validator_callback_ds(void *arg) {
 	}
 
 cleanup:
-	dns_validator_detach(&subvalidator->parent);
-	dns_validator_shutdown(subvalidator);
-	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -711,12 +705,12 @@ static void
 validator_callback_cname(void *arg) {
 	dns_validator_t *subvalidator = (dns_validator_t *)arg;
 	dns_validator_t *val = subvalidator->parent;
-	isc_result_t result;
-	isc_result_t eresult = subvalidator->result;
+	isc_result_t result = subvalidator->result;
 
 	INSIST((val->attributes & VALATTR_INSECURITY) != 0);
 
-	val->subvalidator = NULL;
+	dns_validator_shutdown(val->subvalidator);
+	dns_validator_detach(&val->subvalidator);
 
 	if (CANCELED(val) || CANCELING(val)) {
 		result = ISC_R_CANCELED;
@@ -724,24 +718,21 @@ validator_callback_cname(void *arg) {
 	}
 
 	validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_cname");
-	if (eresult == ISC_R_SUCCESS) {
+	if (result == ISC_R_SUCCESS) {
 		validator_log(val, ISC_LOG_DEBUG(3), "cname with trust %s",
 			      dns_trust_totext(val->frdataset.trust));
 		result = proveunsecure(val, false, true);
 	} else {
-		if (eresult != DNS_R_BROKENCHAIN) {
+		if (result != DNS_R_BROKENCHAIN) {
 			expire_rdatasets(val);
 		}
 		validator_log(val, ISC_LOG_DEBUG(3),
 			      "validator_callback_cname: got %s",
-			      isc_result_totext(eresult));
+			      isc_result_totext(result));
 		result = DNS_R_BROKENCHAIN;
 	}
 
 cleanup:
-	dns_validator_detach(&subvalidator->parent);
-	dns_validator_shutdown(subvalidator);
-	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -756,12 +747,13 @@ static void
 validator_callback_nsec(void *arg) {
 	dns_validator_t *subvalidator = (dns_validator_t *)arg;
 	dns_validator_t *val = subvalidator->parent;
+	dns_name_t *name = subvalidator->name;
 	dns_rdataset_t *rdataset = subvalidator->rdataset;
-	isc_result_t result;
-	isc_result_t eresult = subvalidator->result;
+	isc_result_t result = subvalidator->result;
 	bool exists, data;
 
-	val->subvalidator = NULL;
+	dns_validator_shutdown(subvalidator);
+	dns_validator_detach(&val->subvalidator);
 
 	if (CANCELED(val) || CANCELING(val)) {
 		result = ISC_R_CANCELED;
@@ -769,7 +761,7 @@ validator_callback_nsec(void *arg) {
 	}
 
 	validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_nsec");
-	if (eresult == ISC_R_SUCCESS) {
+	if (result == ISC_R_SUCCESS) {
 		dns_name_t **proofs = val->proofs;
 		dns_name_t *wild = dns_fixedname_name(&val->wild);
 
@@ -777,8 +769,7 @@ validator_callback_nsec(void *arg) {
 		    rdataset->trust == dns_trust_secure &&
 		    (NEEDNODATA(val) || NEEDNOQNAME(val)) &&
 		    !FOUNDNODATA(val) && !FOUNDNOQNAME(val) &&
-		    dns_nsec_noexistnodata(val->type, val->name,
-					   subvalidator->name, rdataset,
+		    dns_nsec_noexistnodata(val->type, val->name, name, rdataset,
 					   &exists, &data, wild, validator_log,
 					   val) == ISC_R_SUCCESS)
 		{
@@ -786,7 +777,7 @@ validator_callback_nsec(void *arg) {
 				val->attributes |= VALATTR_FOUNDNODATA;
 				if (NEEDNODATA(val)) {
 					proofs[DNS_VALIDATOR_NODATAPROOF] =
-						subvalidator->name;
+						name;
 				}
 			}
 			if (!exists) {
@@ -815,7 +806,7 @@ validator_callback_nsec(void *arg) {
 				 */
 				if (NEEDNOQNAME(val)) {
 					proofs[DNS_VALIDATOR_NOQNAMEPROOF] =
-						subvalidator->name;
+						name;
 				}
 			}
 		}
@@ -824,11 +815,10 @@ validator_callback_nsec(void *arg) {
 	} else {
 		validator_log(val, ISC_LOG_DEBUG(3),
 			      "validator_callback_nsec: got %s",
-			      isc_result_totext(eresult));
-		switch (eresult) {
+			      isc_result_totext(result));
+		switch (result) {
 		case ISC_R_CANCELED:
 		case ISC_R_SHUTTINGDOWN:
-			result = eresult;
 			break;
 		case DNS_R_BROKENCHAIN:
 			val->authfail++;
@@ -839,9 +829,6 @@ validator_callback_nsec(void *arg) {
 	}
 
 cleanup:
-	dns_validator_detach(&subvalidator->parent);
-	dns_validator_shutdown(subvalidator);
-	dns_validator_detach(&subvalidator);
 	validate_async_done(val, result);
 }
 
@@ -1794,6 +1781,11 @@ static void
 resume_answer(void *arg) {
 	dns_validator_t *val = arg;
 
+	if (CANCELED(val) || CANCELING(val)) {
+		validate_async_done(val, ISC_R_CANCELED);
+		return;
+	}
+
 	val->resume = true;
 	validate_answer_iter_start(val);
 }
@@ -3507,6 +3499,9 @@ destroy_validator(dns_validator_t *val) {
 	REQUIRE(val->subvalidator == NULL);
 
 	val->magic = 0;
+	if (val->parent != NULL) {
+		dns_validator_detach(&val->parent);
+	}
 	if (val->key != NULL) {
 		dst_key_free(&val->key);
 	}