From a670551b41798910685fd2e3a3d56acad432b2eb Mon Sep 17 00:00:00 2001
From: Libor Peltan <libor.peltan@nic.cz>
Date: Tue, 28 Sep 2021 21:13:07 +0200
Subject: [PATCH] dnskeys: equivalent code refactoring

---
 src/knot/dnssec/key_records.c | 31 ++++++++++++++++++++++++++++++
 src/knot/dnssec/key_records.h |  6 ++++++
 src/knot/dnssec/zone-sign.c   | 36 +++++++++--------------------------
 3 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/src/knot/dnssec/key_records.c b/src/knot/dnssec/key_records.c
index c001ed5463..88b102bd29 100644
--- a/src/knot/dnssec/key_records.c
+++ b/src/knot/dnssec/key_records.c
@@ -34,6 +34,14 @@ void key_records_init(const kdnssec_ctx_t *ctx, key_records_t *r)
 	                KNOT_RRTYPE_RRSIG, KNOT_CLASS_IN, ctx->policy->dnskey_ttl);
 }
 
+void key_records_from_apex(const zone_node_t *apex, key_records_t *r)
+{
+	r->dnskey = node_rrset(apex, KNOT_RRTYPE_DNSKEY);
+	r->cdnskey = node_rrset(apex, KNOT_RRTYPE_CDNSKEY);
+	r->cds = node_rrset(apex, KNOT_RRTYPE_CDS);
+	knot_rrset_init_empty(&r->rrsig);
+}
+
 int key_records_add_rdata(key_records_t *r, uint16_t rrtype, uint8_t *rdata, uint16_t rdlen, uint32_t ttl)
 {
 	knot_rrset_t *to_add;
@@ -77,6 +85,29 @@ void key_records_clear_rdatasets(key_records_t *r)
 	knot_rdataset_clear(&r->rrsig.rrs, NULL);
 }
 
+static int add_one(const knot_rrset_t *rr, changeset_t *ch,
+                   bool rem, changeset_flag_t fl, int ret)
+{
+	if (ret == KNOT_EOK && !knot_rrset_empty(rr)) {
+		if (rem) {
+			ret = changeset_add_removal(ch, rr, fl);
+		} else {
+			ret = changeset_add_addition(ch, rr, fl);
+		}
+	}
+	return ret;
+}
+
+int key_records_to_changeset(const key_records_t *r, changeset_t *ch,
+                             bool rem, changeset_flag_t chfl)
+{
+	int ret = KNOT_EOK;
+	ret = add_one(&r->dnskey,  ch, rem, chfl, ret);
+	ret = add_one(&r->cdnskey, ch, rem, chfl, ret);
+	ret = add_one(&r->cds,     ch, rem, chfl, ret);
+	return ret;
+}
+
 int key_records_dump(char **buf, size_t *buf_size, const key_records_t *r, bool verbose)
 {
 	if (*buf == NULL) {
diff --git a/src/knot/dnssec/key_records.h b/src/knot/dnssec/key_records.h
index 12a82de2f1..0d1c351697 100644
--- a/src/knot/dnssec/key_records.h
+++ b/src/knot/dnssec/key_records.h
@@ -18,15 +18,21 @@
 
 #include "contrib/wire_ctx.h"
 #include "knot/dnssec/zone-keys.h"
+#include "knot/updates/changesets.h"
 
 void key_records_init(const kdnssec_ctx_t *ctx, key_records_t *r);
 
+void key_records_from_apex(const zone_node_t *apex, key_records_t *r);
+
 int key_records_add_rdata(key_records_t *r, uint16_t rrtype, uint8_t *rdata, uint16_t rdlen, uint32_t ttl);
 
 void key_records_clear(key_records_t *r);
 
 void key_records_clear_rdatasets(key_records_t *r);
 
+int key_records_to_changeset(const key_records_t *r, changeset_t *ch,
+                             bool rem, changeset_flag_t chfl);
+
 int key_records_dump(char **buf, size_t *buf_size, const key_records_t *r, bool verbose);
 
 int key_records_sign(const zone_key_t *key, key_records_t *r, const kdnssec_ctx_t *kctx, knot_time_t *expires);
diff --git a/src/knot/dnssec/zone-sign.c b/src/knot/dnssec/zone-sign.c
index 7aec96b099..2822c4578e 100644
--- a/src/knot/dnssec/zone-sign.c
+++ b/src/knot/dnssec/zone-sign.c
@@ -825,11 +825,9 @@ int knot_zone_sign_update_dnskeys(zone_update_t *update,
 	}
 
 	const zone_node_t *apex = update->new_cont->apex;
-	knot_rrset_t dnskeys = node_rrset(apex, KNOT_RRTYPE_DNSKEY);
-	knot_rrset_t cdnskeys = node_rrset(apex, KNOT_RRTYPE_CDNSKEY);
-	knot_rrset_t cdss = node_rrset(apex, KNOT_RRTYPE_CDS);
-	key_records_t add_r;
+	key_records_t add_r, orig_r;
 	memset(&add_r, 0, sizeof(add_r));
+	key_records_from_apex(apex, &orig_r);
 	knot_rrset_t soa = node_rrset(apex, KNOT_RRTYPE_SOA);
 	if (knot_rrset_empty(&soa)) {
 		return KNOT_EINVAL;
@@ -844,11 +842,7 @@ int knot_zone_sign_update_dnskeys(zone_update_t *update,
 #define CHECK_RET if (ret != KNOT_EOK) goto cleanup
 
 	// remove all. This will cancel out with additions later
-	ret = changeset_add_removal(&ch, &dnskeys, 0);
-	CHECK_RET;
-	ret = changeset_add_removal(&ch, &cdnskeys, 0);
-	CHECK_RET;
-	ret = changeset_add_removal(&ch, &cdss, 0);
+	ret = key_records_to_changeset(&orig_r, &ch, true, 0);
 	CHECK_RET;
 
 	// add DNSKEYs, CDNSKEYs and CDSs
@@ -869,24 +863,12 @@ int knot_zone_sign_update_dnskeys(zone_update_t *update,
 	}
 	CHECK_RET;
 
-	if (!knot_rrset_empty(&add_r.cdnskey)) {
-		ret = changeset_add_addition(&ch, &add_r.cdnskey, CHANGESET_CHECK);
-		CHECK_RET;
-	}
-
-	if (!knot_rrset_empty(&add_r.cds)) {
-		ret = changeset_add_addition(&ch, &add_r.cds, CHANGESET_CHECK);
-		if (dnssec_ctx->policy->ds_push && node_rrtype_exists(ch.add->apex, KNOT_RRTYPE_CDS)) {
-			// there is indeed a change to CDS
-			update->zone->timers.next_ds_push = time(NULL) + dnssec_ctx->policy->propagation_delay;
-			zone_events_schedule_at(update->zone, ZONE_EVENT_DS_PUSH, update->zone->timers.next_ds_push);
-		}
-		CHECK_RET;
-	}
-
-	if (!knot_rrset_empty(&add_r.dnskey)) {
-		ret = changeset_add_addition(&ch, &add_r.dnskey, CHANGESET_CHECK);
-		CHECK_RET;
+	ret = key_records_to_changeset(&add_r, &ch, false, CHANGESET_CHECK);
+	CHECK_RET;
+	if (dnssec_ctx->policy->ds_push && node_rrtype_exists(ch.add->apex, KNOT_RRTYPE_CDS)) {
+		// there is indeed a change to CDS
+		update->zone->timers.next_ds_push = time(NULL) + dnssec_ctx->policy->propagation_delay;
+		zone_events_schedule_at(update->zone, ZONE_EVENT_DS_PUSH, update->zone->timers.next_ds_push);
 	}
 
 	if (!knot_rrset_empty(&add_r.rrsig)) {
-- 
GitLab