changeset: DNSSEC and IXFR fixes, more freeing functions.

src/knot/nameserver/ixfr.c

@@ -152,7 +152,7 @@ static int ixfr_load_chsets(list_t *chgsets, const zone_t *zone,
 
 	ret = journal_load_changesets(zone, chgsets, serial_from, serial_to);
 	if (ret != KNOT_EOK) {
-		changesets_free(chgsets, NULL);
+		changesets_free(chgsets);
 	}
 
 	return ret;
@@ -190,7 +190,7 @@ static void ixfr_answer_cleanup(struct query_data *qdata)
 	ptrlist_free(&ixfr->proc.nodes, mm);
 	changeset_iter_free(ixfr->cur, NULL);
 	ixfr->cur = NULL;
-	changesets_free(&ixfr->changesets, NULL);
+	changesets_free(&ixfr->changesets);
 	mm->free(qdata->ext);
 
 	/* Allow zone changes (finished). */
@@ -223,17 +223,18 @@ static int ixfr_answer_init(struct query_data *qdata)
 	mm_ctx_t *mm = qdata->mm;
 	struct ixfr_proc *xfer = mm->alloc(mm->ctx, sizeof(struct ixfr_proc));
 	if (xfer == NULL) {
-		changesets_free(&chgsets, NULL);
+		changesets_free(&chgsets);
 		return KNOT_ENOMEM;
 	}
 	memset(xfer, 0, sizeof(struct ixfr_proc));
 	gettimeofday(&xfer->proc.tstamp, NULL);
 	xfer->state = IXFR_SOA_DEL;
 	init_list(&xfer->proc.nodes);
+	init_list(&xfer->changesets);
+	add_tail_list(&xfer->changesets, &chgsets);
 	xfer->qdata = qdata;
 
 	/* Put all changesets to processing queue. */
-	xfer->changesets = chgsets;
 	changeset_t *chs = NULL;
 	WALK_LIST(chs, xfer->changesets) {
 		ptrlist_add(&xfer->proc.nodes, chs, mm);
@@ -297,7 +298,7 @@ static void ixfrin_cleanup(struct answer_data *data)
 {
 	struct ixfr_proc *proc = data->ext;
 	if (proc) {
-		changesets_free(&proc->changesets, data->mm);
+		changesets_clear(&proc->changesets);
 		mm_free(data->mm, proc);
 		data->ext = NULL;
 	}
@@ -332,7 +333,7 @@ static int ixfrin_finalize(struct answer_data *adata)
 
 	assert(ixfr->state == IXFR_DONE);
 	int ret = zone_change_apply_and_store(&ixfr->changesets,
-	                                      ixfr->zone, "IXFR", adata->mm);
+	                                      ixfr->zone, "IXFR");
 	if (ret != KNOT_EOK) {
 		IXFRIN_LOG(LOG_ERR, "Failed to apply changes to zone - %s",
 		           knot_strerror(ret));
@@ -381,7 +382,7 @@ static int solve_soa_del(const knot_rrset_t *rr, struct ixfr_proc *proc)
 	// Store SOA into changeset.
 	change->soa_from = knot_rrset_copy(rr, proc->mm);
 	if (change->soa_from == NULL) {
-		changeset_clear(change, NULL);
+		changeset_clear(change);
 		return KNOT_ENOMEM;
 	}
 

src/knot/nameserver/update.c

@@ -106,7 +106,7 @@ static int sign_update(zone_t *zone, const zone_contents_t *old_contents,
 		                                 &refresh_at);
 	}
 	if (ret != KNOT_EOK) {
-		changeset_clear(&sec_ch, NULL);
+		changeset_clear(&sec_ch);
 		return ret;
 	}
 
@@ -116,13 +116,13 @@ static int sign_update(zone_t *zone, const zone_contents_t *old_contents,
 	add_head(&changes, &sec_ch.n);
 	ret = apply_changesets_directly(new_contents, &changes);
 	if (ret != KNOT_EOK) {
-		changeset_clear(&sec_ch, NULL);
+		changeset_clear(&sec_ch);
 		return ret;
 	}
 
 	// Merge changesets
 	ret = changeset_merge(ddns_ch, &sec_ch);
-	changeset_clear(&sec_ch, NULL);
+	changeset_clear(&sec_ch);
 	if (ret != KNOT_EOK) {
 		return ret;
 	}
@@ -156,7 +156,7 @@ static int process_authenticated(uint16_t *rcode, struct query_data *qdata)
 	ret = ddns_process_update(zone, query, &ddns_ch, rcode);
 	if (ret != KNOT_EOK) {
 		assert(*rcode != KNOT_RCODE_NOERROR);
-		changeset_clear(&ddns_ch, NULL);
+		changeset_clear(&ddns_ch);
 		return ret;
 	}
 	assert(*rcode == KNOT_RCODE_NOERROR);
@@ -174,11 +174,11 @@ static int process_authenticated(uint16_t *rcode, struct query_data *qdata)
 			} else {
 				*rcode = KNOT_RCODE_SERVFAIL;
 			}
-			changeset_clear(&ddns_ch, NULL);
+			changeset_clear(&ddns_ch);
 			return ret;
 		}
 	} else {
-		changeset_clear(&ddns_ch, NULL);
+		changeset_clear(&ddns_ch);
 		*rcode = KNOT_RCODE_NOERROR;
 		return KNOT_EOK;
 	}
@@ -188,7 +188,7 @@ static int process_authenticated(uint16_t *rcode, struct query_data *qdata)
 		ret = sign_update(zone, zone->contents, new_contents, &ddns_ch);
 		if (ret != KNOT_EOK) {
 			update_rollback(&apply, &new_contents);
-			changeset_clear(&ddns_ch, NULL);
+			changeset_clear(&ddns_ch);
 			*rcode = KNOT_RCODE_SERVFAIL;
 			return ret;
 		}
@@ -198,7 +198,7 @@ static int process_authenticated(uint16_t *rcode, struct query_data *qdata)
 	ret = zone_change_store(zone, &apply);
 	if (ret != KNOT_EOK) {
 		update_rollback(&apply, &new_contents);
-		changeset_clear(&ddns_ch, NULL);
+		changeset_clear(&ddns_ch);
 		*rcode = KNOT_RCODE_SERVFAIL;
 		return ret;
 	}
@@ -209,7 +209,7 @@ static int process_authenticated(uint16_t *rcode, struct query_data *qdata)
 	update_free_old_zone(&old_contents);
 
 	update_cleanup(&apply);
-	changeset_clear(&ddns_ch, NULL);
+	changeset_clear(&ddns_ch);
 
 	/* Sync zonefile immediately if configured. */
 	if (zone->conf->dbsync_timeout == 0) {

src/knot/server/journal.c

@@ -956,6 +956,7 @@ static int changesets_unpack(changeset_t *chs)
 
 	assert(rrset.type == KNOT_RRTYPE_SOA);
 	chs->soa_from = knot_rrset_copy(&rrset, NULL);
+	knot_rrset_clear(&rrset, NULL);
 	if (chs->soa_from == NULL) {
 		return KNOT_ENOMEM;
 	}
@@ -965,8 +966,8 @@ static int changesets_unpack(changeset_t *chs)
 	while (remaining > 0) {
 
 		/* Parse next RRSet. */
-		knot_rrset_init_empty(&rrset);
 		stream = chs->data + (chs->size - remaining);
+		knot_rrset_init_empty(&rrset);
 		ret = rrset_deserialize(stream, &remaining, &rrset);
 		if (ret != KNOT_EOK) {
 			return KNOT_EMALF;
@@ -978,6 +979,7 @@ static int changesets_unpack(changeset_t *chs)
 			if (in_remove_section) {
 				chs->soa_to = knot_rrset_copy(&rrset, NULL);
 				if (chs->soa_to == NULL) {
+					knot_rrset_clear(&rrset, NULL);
 					return KNOT_ENOMEM;
 				}
 				in_remove_section = false;
@@ -994,8 +996,10 @@ static int changesets_unpack(changeset_t *chs)
 				ret = changeset_add_rrset(chs, &rrset);
 			}
 		}
+		knot_rrset_clear(&rrset, NULL);
 	}
 
+	knot_rrset_clear(&rrset, NULL);
 	return ret;
 }
 
@@ -1153,9 +1157,6 @@ static int load_changeset(journal_t *journal, journal_node_t *n, const zone_t *z
 		return KNOT_ENOMEM;
 	}
 
-	/* Insert into changeset list. */
-	add_tail(chgs, &ch->n);
-
 	/* Initialize changeset. */
 	ch->data = malloc(n->len);
 	if (!ch->data) {
@@ -1170,6 +1171,10 @@ static int load_changeset(journal_t *journal, journal_node_t *n, const zone_t *z
 
 	/* Update changeset binary size. */
 	ch->size = n->len;
+
+	/* Insert into changeset list. */
+	add_tail(chgs, &ch->n);
+
 	return KNOT_EOK;
 }
 

src/knot/updates/changesets.c

@@ -156,7 +156,7 @@ int changeset_merge(changeset_t *ch1, changeset_t *ch2)
 	return KNOT_EOK;
 }
 
-void changeset_clear(changeset_t *ch, mm_ctx_t *rr_mm)
+void changeset_clear(changeset_t *ch)
 {
 	if (ch == NULL) {
 		return;
@@ -166,21 +166,40 @@ void changeset_clear(changeset_t *ch, mm_ctx_t *rr_mm)
 	zone_contents_deep_free(&ch->add);
 	zone_contents_deep_free(&ch->remove);
 
-	knot_rrset_free(&ch->soa_from, rr_mm);
-	knot_rrset_free(&ch->soa_to, rr_mm);
+	knot_rrset_free(&ch->soa_from, NULL);
+	knot_rrset_free(&ch->soa_to, NULL);
 
 	// Delete binary data
 	free(ch->data);
 }
 
-void changesets_free(list_t *chgs, mm_ctx_t *rr_mm)
+void changeset_free(changeset_t *ch)
+{
+	changeset_clear(ch);
+	free(ch);
+}
+
+void changesets_clear(list_t *chgs)
+{
+	if (chgs) {
+		changeset_t *chg, *nxt;
+		WALK_LIST_DELSAFE(chg, nxt, *chgs) {
+			changeset_clear(chg);
+			rem_node(&chg->n);
+		}
+		init_list(chgs);
+	}
+}
+
+void changesets_free(list_t *chgs)
 {
 	if (chgs) {
 		changeset_t *chg, *nxt;
 		WALK_LIST_DELSAFE(chg, nxt, *chgs) {
-			changeset_clear(chg, rr_mm);
 			rem_node(&chg->n);
+			changeset_free(chg);
 		}
+		init_list(chgs);
 	}
 }
 

src/knot/updates/changesets.h

@@ -50,18 +50,11 @@ typedef struct changeset {
 typedef struct {
 	list_t iters;
 	const zone_node_t *node;
-	int32_t node_pos;
+	uint16_t node_pos;
 } changeset_iter_t;
 
 /*----------------------------------------------------------------------------*/
 
-typedef enum {
-	CHANGESET_ADD, /*!< Put RR into 'add' section. */
-	CHANGESET_REMOVE /*!< Put RR into 'remove' section. */
-} changeset_part_t;
-
-/*----------------------------------------------------------------------------*/
-
 changeset_t *changeset_new(mm_ctx_t *mm, const knot_dname_t *apex);
 void changeset_init(changeset_t *ch, const knot_dname_t *apex, mm_ctx_t *mm);
 
@@ -107,8 +100,10 @@ size_t changeset_size(const changeset_t *ch);
  * \param changesets  Double pointer to changesets structure to be freed.
  * \param mm          Memory context used to allocate RRSets.
  */
-void changesets_free(list_t *chgs, mm_ctx_t *rr_mm);
-void changeset_clear(changeset_t *ch, mm_ctx_t *rr_mm);
+void changesets_clear(list_t *chgs);
+void changesets_free(list_t *chgs);
+void changeset_clear(changeset_t *ch);
+void changeset_free(changeset_t *ch);
 
 int changeset_merge(changeset_t *ch1, changeset_t *ch2);
 changeset_iter_t *changeset_iter_add(const changeset_t *ch, bool sorted);

src/knot/zone/events.c

@@ -538,7 +538,7 @@ static int event_dnssec(zone_t *zone)
 		list_t apply;
 		init_list(&apply);
 		add_head(&apply, &ch.n);
-		ret = zone_change_apply_and_store(&apply, zone, "DNSSEC", NULL);
+		ret = zone_change_apply_and_store(&apply, zone, "DNSSEC");
 		if (ret != KNOT_EOK) {
 			log_zone_error("%s Could not sign zone (%s).\n",
 				       msgpref, knot_strerror(ret));
@@ -555,7 +555,7 @@ static int event_dnssec(zone_t *zone)
 	}
 
 done:
-	changeset_clear(&ch, NULL);
+	changeset_clear(&ch);
 	free(msgpref);
 	return ret;
 }

src/knot/zone/zone-load.c

@@ -97,7 +97,7 @@ int zone_load_journal(zone_t *zone)
 	init_list(&chgs);
 	int ret = journal_load_changesets(zone, &chgs, serial, serial - 1);
 	if ((ret != KNOT_EOK && ret != KNOT_ERANGE) || EMPTY_LIST(chgs)) {
-		changesets_free(&chgs, NULL);
+		changesets_free(&chgs);
 		/* Absence of records is not an error. */
 		if (ret == KNOT_ENOENT) {
 			return KNOT_EOK;
@@ -113,7 +113,7 @@ int zone_load_journal(zone_t *zone)
 	              serial, zone_contents_serial(zone->contents),
 	              knot_strerror(ret));
 
-	changesets_free(&chgs, NULL);
+	changesets_free(&chgs);
 	return ret;
 }
 
@@ -134,7 +134,7 @@ int zone_load_post(zone_contents_t *contents, zone_t *zone, uint32_t *dnssec_ref
 		ret = knot_dnssec_zone_sign(contents, conf, &ch, KNOT_SOA_SERIAL_UPDATE,
 		                            dnssec_refresh);
 		if (ret != KNOT_EOK) {
-			changeset_clear(&ch, NULL);
+			changeset_clear(&ch);
 			return ret;
 		}
 
@@ -143,7 +143,7 @@ int zone_load_post(zone_contents_t *contents, zone_t *zone, uint32_t *dnssec_ref
 		init_list(&apply);
 		add_head(&apply, &ch.n);
 		ret = zone_change_commit(contents, &apply);
-		changeset_clear(&ch, NULL);
+		changeset_clear(&ch);
 		if (ret != KNOT_EOK) {
 			return ret;
 		}
@@ -160,21 +160,21 @@ int zone_load_post(zone_contents_t *contents, zone_t *zone, uint32_t *dnssec_ref
 			                 "but serial didn't - won't "
 			                 "create journal entry.\n",
 			                 conf->name);
-			changeset_clear(&diff_change, NULL);
+			changeset_clear(&diff_change);
 			ret = KNOT_EOK;
 		} else if (ret == KNOT_ERANGE) {
 			log_zone_warning("Zone %s: Zone file changed, "
 			                 "but serial is lower than before - "
 			                 "IXFR history will be lost.\n",
 			                 conf->name);
-			changeset_clear(&diff_change, NULL);
+			changeset_clear(&diff_change);
 			ret = KNOT_EOK;
 		} else if (ret != KNOT_EOK) {
 			log_zone_error("Zone %s: Failed to calculate "
 			               "differences from the zone "
 			               "file update: %s\n",
 			               conf->name, knot_strerror(ret));
-			changeset_clear(&diff_change, NULL);
+			changeset_clear(&diff_change);
 			return ret;
 		}
 	}
@@ -186,7 +186,7 @@ int zone_load_post(zone_contents_t *contents, zone_t *zone, uint32_t *dnssec_ref
 		add_head(&apply, &diff_change.n);
 		ret = zone_change_store(zone, &apply);
 	}
-	changeset_clear(&diff_change, NULL);
+	changeset_clear(&diff_change);
 
 	return ret;
 }

src/knot/zone/zone.c

@@ -143,8 +143,7 @@ int zone_change_store(zone_t *zone, list_t *chgs)
 /*! \note @mvavrusa Moved from zones.c, this needs a common API. */
 int zone_change_apply_and_store(list_t *chgs,
                                 zone_t *zone,
-                                const char *msgpref,
-                                mm_ctx_t *rr_mm)
+                                const char *msgpref)
 {
 	int ret = KNOT_EOK;
 
@@ -152,7 +151,6 @@ int zone_change_apply_and_store(list_t *chgs,
 	ret = apply_changesets(zone, chgs, &new_contents);
 	if (ret != KNOT_EOK) {
 		log_zone_error("%s Failed to apply changesets.\n", msgpref);
-		changesets_free(chgs, rr_mm);
 		return ret;
 	}
 
@@ -161,7 +159,6 @@ int zone_change_apply_and_store(list_t *chgs,
 	if (ret != KNOT_EOK) {
 		log_zone_error("%s Failed to store changesets.\n", msgpref);
 		update_rollback(chgs, &new_contents);
-		changesets_free(chgs, rr_mm);
 		return ret;
 	}
 
@@ -170,9 +167,7 @@ int zone_change_apply_and_store(list_t *chgs,
 	synchronize_rcu();
 	update_free_old_zone(&old_contents);
 
-	/* Free changesets, but not the data. */
 	update_cleanup(chgs);
-	changesets_free(chgs, rr_mm);
 
 	return KNOT_EOK;
 }

src/knot/zone/zone.h

@@ -98,8 +98,7 @@ int zone_change_store(zone_t *zone, list_t *chgs);
 /*! \note @mvavrusa Moved from zones.c, this needs a common API. */
 int zone_change_apply_and_store(list_t *chs,
                                 zone_t *zone,
-                                const char *msgpref,
-                                mm_ctx_t *rr_mm);
+                                const char *msgpref);
 /*!
  * \brief Atomically switch the content of the zone.
  */

tests-extra/tools/dnstest/params.py

@@ -35,7 +35,7 @@ debug = False
 valgrind_bin = get_binary("KNOT_TEST_VALGRIND", "valgrind")
 # KNOT_TEST_VALGRIND_FLAGS - valgrind flags.
 valgrind_flags = get_param("KNOT_TEST_VALGRIND_FLAGS",
-                           "--leak-check=full --vgdb=yes")
+                           "--leak-check=full --track-origins=yes --vgdb=yes")
 # KNOT_TEST_GDB - gdb binary.
 gdb_bin = get_binary("KNOT_TEST_GDB", "gdb")
 # KNOT_TEST_VGDB - vgdb binary.

tests/changeset.c

@@ -145,7 +145,7 @@ int main(int argc, char *argv[])
 	list_t chgs;
 	init_list(&chgs);
 	add_head(&chgs, &ch2->n);
-	changesets_free(&chgs, NULL);
+	changesets_clear(&chgs, NULL);
 	ok(changeset_empty(ch2), "changeset: clear list");
 	free(ch2);