Configuration manager expriments

lib/isccfg/cfgmgr.README contains all explainations about those
changes.

What is probably more interesting right now is to look at the
lib/isccfg/cfgmgr.h to get an idea of the interface proposal and few
changes to try it in bin/named/server.c.

Tested and working system tests: views, dlzexternal, rpz, resolver

Ongoing: rpz changes

bin/named/config.c

@@ -40,6 +40,7 @@
 
 #include <isccfg/grammar.h>
 #include <isccfg/namedconf.h>
+#include <isccfg/cfgmgr.h>
 
 #include <named/config.h>
 #include <named/globals.h>
@@ -644,12 +645,12 @@ named_config_getipandkeylist(const cfg_obj_t *config, const char *listtype,
 	/*
 	 * Get system defaults.
 	 */
-	result = named_config_getport(config, "port", &def_port);
+	result = named_config_getport("port", &def_port);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup;
 	}
 
-	result = named_config_getport(config, "tls-port", &def_tlsport);
+	result = named_config_getport("tls-port", &def_tlsport);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup;
 	}
@@ -871,30 +872,19 @@ cleanup:
 }
 
 isc_result_t
-named_config_getport(const cfg_obj_t *config, const char *type,
-		     in_port_t *portp) {
-	const cfg_obj_t *maps[3];
-	const cfg_obj_t *options = NULL;
-	const cfg_obj_t *portobj = NULL;
-	isc_result_t result;
-	int i;
+named_config_getport(const char *type, in_port_t *portp) {
+	cfgmgr_it_t *options = NULL;
+	uint32_t port;
 
-	(void)cfg_map_get(config, "options", &options);
-	i = 0;
-	if (options != NULL) {
-		maps[i++] = options;
-	}
-	maps[i++] = named_g_defaults;
-	maps[i] = NULL;
+	REQUIRE(cfgmgr_getit(NULL, &options, "options") == ISC_R_SUCCESS);
+	REQUIRE(cfgmgr_getuint32(options, type, &port) == ISC_R_SUCCESS);
+	cfgmgr_putit(&options);
 
-	result = named_config_get(maps, type, &portobj);
-	INSIST(result == ISC_R_SUCCESS);
-	if (cfg_obj_asuint32(portobj) >= UINT16_MAX) {
-		cfg_obj_log(portobj, ISC_LOG_ERROR, "port '%u' out of range",
-			    cfg_obj_asuint32(portobj));
+	if (port >= UINT16_MAX) {
 		return ISC_R_RANGE;
 	}
-	*portp = (in_port_t)cfg_obj_asuint32(portobj);
+	*portp = (in_port_t)port;
+
 	return ISC_R_SUCCESS;
 }
 

bin/named/include/named/config.h

@@ -62,8 +62,7 @@ named_config_getipandkeylist(const cfg_obj_t *config, const char *listtype,
 			     dns_ipkeylist_t *ipkl);
 
 isc_result_t
-named_config_getport(const cfg_obj_t *config, const char *type,
-		     in_port_t *portp);
+named_config_getport(const char *type, in_port_t *portp);
 
 isc_result_t
 named_config_getkeyalgorithm(const char *str, unsigned int *typep,

bin/named/server.c

@@ -109,6 +109,7 @@
 
 #include <dst/dst.h>
 
+#include <isccfg/cfgmgr.h>
 #include <isccfg/check.h>
 #include <isccfg/grammar.h>
 #include <isccfg/kaspconf.h>
@@ -465,8 +466,7 @@ configure_forward(const cfg_obj_t *config, dns_view_t *view,
 		  const cfg_obj_t *forwardtype);
 
 static isc_result_t
-configure_alternates(const cfg_obj_t *config, dns_view_t *view,
-		     const cfg_obj_t *alternates);
+configure_alternates(dns_view_t *view, const cfg_obj_t *alternates);
 
 static isc_result_t
 configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
@@ -1268,42 +1268,37 @@ cleanup:
  * Get a dispatch appropriate for the resolver of a given view.
  */
 static isc_result_t
-get_view_querysource_dispatch(const cfg_obj_t **maps, int af,
+get_view_querysource_dispatch(cfgmgr_it_t *viewit, int af,
 			      dns_dispatch_t **dispatchp) {
 	isc_result_t result = ISC_R_FAILURE;
 	dns_dispatch_t *disp = NULL;
 	isc_sockaddr_t sa;
-	const cfg_obj_t *obj = NULL;
+	const char *querysource = "query-source";
+	const char *querysource6 = "query-source-v6";
 
 	switch (af) {
 	case AF_INET:
-		result = named_config_get(maps, "query-source", &obj);
+		result = cfgmgr_getnone(viewit, querysource);
+		if (result == ISC_R_SUCCESS) {
+			return result;
+		}
+		result = cfgmgr_getsockaddr(viewit, querysource, &sa);
 		INSIST(result == ISC_R_SUCCESS);
 		break;
 	case AF_INET6:
-		result = named_config_get(maps, "query-source-v6", &obj);
+		result = cfgmgr_getnone(viewit, querysource6);
+		if (result == ISC_R_SUCCESS) {
+			return result;
+		}
+		result = cfgmgr_getsockaddr(viewit, querysource6, &sa);
 		INSIST(result == ISC_R_SUCCESS);
 		break;
 	default:
 		UNREACHABLE();
 	}
 
-	if (cfg_obj_isvoid(obj)) {
-		/*
-		 * We don't want to use this address family, let's
-		 * bail now. The dispatch object for this family will
-		 * be null then not used to run queries.
-		 */
-		return ISC_R_SUCCESS;
-	} else {
-		/*
-		 * obj _has_ to be sockaddr here, cfg_obj_assockaddr()
-		 * asserts this internally.
-		 */
-		sa = *(cfg_obj_assockaddr(obj));
-		INSIST(isc_sockaddr_pf(&sa) == af);
-		INSIST(isc_sockaddr_getport(&sa) == 0);
-	}
+	INSIST(isc_sockaddr_pf(&sa) == af);
+	INSIST(isc_sockaddr_getport(&sa) == 0);
 
 	/*
 	 * If we don't support this address family, we're done!
@@ -2014,8 +2009,9 @@ configure_rpz_name2(dns_view_t *view, const cfg_obj_t *obj, dns_name_t *name,
 
 static isc_result_t
 configure_rpz_zone(dns_view_t *view, const cfg_listelt_t *element,
-		   bool recursive_only_default, bool add_soa_default,
-		   dns_ttl_t ttl_default, uint32_t minupdateinterval_default,
+		   const cfgmgr_it_t *zoneit, bool recursive_only_default,
+		   bool add_soa_default, dns_ttl_t ttl_default,
+		   uint32_t minupdateinterval_default,
 		   const dns_rpz_zone_t *old, bool *old_rpz_okp) {
 	const cfg_obj_t *rpz_obj, *obj;
 	const char *str;
@@ -2042,6 +2038,7 @@ configure_rpz_zone(dns_view_t *view, const cfg_listelt_t *element,
 		return result;
 	}
 
+	
 	obj = cfg_tuple_get(rpz_obj, "recursive-only");
 	if (cfg_obj_isvoid(obj) ? recursive_only_default
 				: cfg_obj_asboolean(obj))
@@ -2051,27 +2048,30 @@ configure_rpz_zone(dns_view_t *view, const cfg_listelt_t *element,
 		view->rpzs->p.no_rd_ok |= DNS_RPZ_ZBIT(zone->num);
 	}
 
-	obj = cfg_tuple_get(rpz_obj, "log");
-	if (!cfg_obj_isvoid(obj) && !cfg_obj_asboolean(obj)) {
+	bool value;
+	result = cfgmgr_getbool(zoneit, "log", &value);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(zoneit, "log");
+		value = result == ISC_R_SUCCESS;
+	}
+	if (!value) {
 		view->rpzs->p.no_log |= DNS_RPZ_ZBIT(zone->num);
 	} else {
 		view->rpzs->p.no_log &= ~DNS_RPZ_ZBIT(zone->num);
 	}
 
-	obj = cfg_tuple_get(rpz_obj, "max-policy-ttl");
-	if (cfg_obj_isduration(obj)) {
-		zone->max_policy_ttl = cfg_obj_asduration(obj);
-	} else {
+	result = cfgmgr_getduration(zoneit, "max-policy-ttl",
+				    &zone->max_policy_ttl);
+	if (result != ISC_R_SUCCESS) {
 		zone->max_policy_ttl = ttl_default;
 	}
 	if (*old_rpz_okp && zone->max_policy_ttl != old->max_policy_ttl) {
 		*old_rpz_okp = false;
 	}
 
-	obj = cfg_tuple_get(rpz_obj, "min-update-interval");
-	if (cfg_obj_isduration(obj)) {
-		zone->min_update_interval = cfg_obj_asduration(obj);
-	} else {
+	result = cfgmgr_getduration(zoneit, "min-update-interval",
+				    &zone->min_update_interval);
+	if (result != ISC_R_SUCCESS) {
 		zone->min_update_interval = minupdateinterval_default;
 	}
 	if (*old_rpz_okp &&
@@ -2194,114 +2194,103 @@ configure_rpz_zone(dns_view_t *view, const cfg_listelt_t *element,
 }
 
 static isc_result_t
-configure_rpz(dns_view_t *view, dns_view_t *pview, const cfg_obj_t *rpz_obj,
+configure_rpz(cfgmgr_it_t *ctxit, dns_view_t *view, dns_view_t *pview,
+	      const cfg_obj_t *rpz_obj,
 	      bool *old_rpz_okp) {
 	const cfg_listelt_t *zone_element;
-	const cfg_obj_t *sub_obj;
 	bool recursive_only_default, add_soa_default;
-	bool nsip_enabled, nsdname_enabled;
+	bool nsip_enabled = true;
+	bool nsdname_enabled = true;
 	dns_rpz_zbits_t nsip_on, nsdname_on;
-	dns_ttl_t ttl_default;
-	uint32_t minupdateinterval_default;
+	dns_ttl_t ttl_default = DNS_RPZ_MAX_TTL_DEFAULT;
+	uint32_t minupdateinterval_default = DNS_RPZ_MINUPDATEINTERVAL_DEFAULT;
 	dns_rpz_zones_t *zones;
 	const dns_rpz_zones_t *old;
 	bool pview_must_detach = false;
 	const dns_rpz_zone_t *old_zone;
 	isc_result_t result;
 	int i;
+	cfgmgr_it_t *rpzit = NULL;
+	cfgmgr_it_t *zoneit = NULL;
+	bool hasmorezone = true;
 
 	*old_rpz_okp = false;
 
+	result = cfgmgr_getit(ctxit, &rpzit, "response-policy");
+	REQUIRE(result == ISC_R_SUCCESS);
+
+	result = cfgmgr_getit(rpzit, &zoneit, "zone list");
+	if (result != ISC_R_SUCCESS) {
+		result = ISC_R_SUCCESS;
+		goto out;
+	}
+
 	zone_element = cfg_list_first(cfg_tuple_get(rpz_obj, "zone list"));
 	if (zone_element == NULL) {
-		return ISC_R_SUCCESS;
+		result = ISC_R_SUCCESS;
+		goto out;
 	}
 
-	nsip_enabled = true;
-	sub_obj = cfg_tuple_get(rpz_obj, "nsip-enable");
-	if (!cfg_obj_isvoid(sub_obj)) {
-		nsip_enabled = cfg_obj_asboolean(sub_obj);
-	}
+	(void)cfgmgr_getbool(rpzit, "nsip-enable", &nsip_enabled);
 	nsip_on = nsip_enabled ? DNS_RPZ_ALL_ZBITS : 0;
 
-	nsdname_enabled = true;
-	sub_obj = cfg_tuple_get(rpz_obj, "nsdname-enable");
-	if (!cfg_obj_isvoid(sub_obj)) {
-		nsdname_enabled = cfg_obj_asboolean(sub_obj);
-	}
+	(void)cfgmgr_getbool(rpzit, "nsdname-enable", &nsdname_enabled);
 	nsdname_on = nsdname_enabled ? DNS_RPZ_ALL_ZBITS : 0;
 
 	result = dns_rpz_new_zones(view, named_g_loopmgr, &view->rpzs);
 	if (result != ISC_R_SUCCESS) {
-		return result;
+		goto out;
 	}
 
 	zones = view->rpzs;
 
 	zones->p.nsip_on = nsip_on;
 	zones->p.nsdname_on = nsdname_on;
+	zones->p.min_ns_labels = 2;
 
-	sub_obj = cfg_tuple_get(rpz_obj, "recursive-only");
-	if (!cfg_obj_isvoid(sub_obj) && !cfg_obj_asboolean(sub_obj)) {
-		recursive_only_default = false;
-	} else {
-		recursive_only_default = true;
+	result = cfgmgr_getbool(rpzit, "recursive-only",
+				&recursive_only_default);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "recursive-only");
+		recursive_only_default = result == ISC_R_SUCCESS;
 	}
 
-	sub_obj = cfg_tuple_get(rpz_obj, "add-soa");
-	if (!cfg_obj_isvoid(sub_obj) && !cfg_obj_asboolean(sub_obj)) {
-		add_soa_default = false;
-	} else {
-		add_soa_default = true;
+	result = cfgmgr_getbool(rpzit, "add-soa", &add_soa_default);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "add-soa");
+		add_soa_default = result == ISC_R_SUCCESS;
 	}
 
-	sub_obj = cfg_tuple_get(rpz_obj, "break-dnssec");
-	if (!cfg_obj_isvoid(sub_obj) && cfg_obj_asboolean(sub_obj)) {
-		zones->p.break_dnssec = true;
-	} else {
-		zones->p.break_dnssec = false;
+	result = cfgmgr_getbool(rpzit, "break-dnssec", &zones->p.break_dnssec);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "break-dnssec");
+		zones->p.break_dnssec = result != ISC_R_SUCCESS;
 	}
 
-	sub_obj = cfg_tuple_get(rpz_obj, "max-policy-ttl");
-	if (cfg_obj_isduration(sub_obj)) {
-		ttl_default = cfg_obj_asduration(sub_obj);
-	} else {
-		ttl_default = DNS_RPZ_MAX_TTL_DEFAULT;
+	(void)cfgmgr_getduration(rpzit, "max-policy-ttl", &ttl_default);
+	(void)cfgmgr_getduration(rpzit, "min-update-interval",
+				 &minupdateinterval_default);
+	(void)cfgmgr_getuint32(rpzit, "min-ns-dots", &zones->p.min_ns_labels);
+ 
+	result = cfgmgr_getbool(rpzit, "qname-wait-recurse",
+				&zones->p.qname_wait_recurse);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "qname-wait-recurse");
+		zones->p.qname_wait_recurse = result == ISC_R_SUCCESS;
 	}
 
-	sub_obj = cfg_tuple_get(rpz_obj, "min-update-interval");
-	if (cfg_obj_isduration(sub_obj)) {
-		minupdateinterval_default = cfg_obj_asduration(sub_obj);
-	} else {
-		minupdateinterval_default = DNS_RPZ_MINUPDATEINTERVAL_DEFAULT;
+	result = cfgmgr_getbool(rpzit, "nsdname-wait-recurse",
+				 &zones->p.nsdname_wait_recurse);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "nsdname-wait-recurse");
+		zones->p.nsdname_wait_recurse = result == ISC_R_SUCCESS;
 	}
 
-	sub_obj = cfg_tuple_get(rpz_obj, "min-ns-dots");
-	if (cfg_obj_isuint32(sub_obj)) {
-		zones->p.min_ns_labels = cfg_obj_asuint32(sub_obj) + 1;
-	} else {
-		zones->p.min_ns_labels = 2;
-	}
-
-	sub_obj = cfg_tuple_get(rpz_obj, "qname-wait-recurse");
-	if (cfg_obj_isvoid(sub_obj) || cfg_obj_asboolean(sub_obj)) {
-		zones->p.qname_wait_recurse = true;
-	} else {
-		zones->p.qname_wait_recurse = false;
-	}
-
-	sub_obj = cfg_tuple_get(rpz_obj, "nsdname-wait-recurse");
-	if (cfg_obj_isvoid(sub_obj) || cfg_obj_asboolean(sub_obj)) {
-		zones->p.nsdname_wait_recurse = true;
-	} else {
-		zones->p.nsdname_wait_recurse = false;
-	}
-
-	sub_obj = cfg_tuple_get(rpz_obj, "nsip-wait-recurse");
-	if (cfg_obj_isvoid(sub_obj) || cfg_obj_asboolean(sub_obj)) {
-		zones->p.nsip_wait_recurse = true;
-	} else {
-		zones->p.nsip_wait_recurse = false;
+	result = cfgmgr_getbool(rpzit, "nsip-wait-recurse",
+				 &zones->p.nsip_wait_recurse);
+	if (result != ISC_R_SUCCESS) {
+		result = cfgmgr_getnone(rpzit, "nsip-wait-recurse");
+		zones->p.nsip_wait_recurse = result == ISC_R_SUCCESS;
 	}
 
 	if (pview != NULL) {
@@ -2326,6 +2315,7 @@ configure_rpz(dns_view_t *view, dns_view_t *pview, const cfg_obj_t *rpz_obj,
 	for (i = 0; zone_element != NULL;
 	     ++i, zone_element = cfg_list_next(zone_element))
 	{
+		REQUIRE(hasmorezone);
 		INSIST(!*old_rpz_okp || old != NULL);
 		if (*old_rpz_okp && i < old->p.num_zones) {
 			old_zone = old->zones[i];
@@ -2334,15 +2324,17 @@ configure_rpz(dns_view_t *view, dns_view_t *pview, const cfg_obj_t *rpz_obj,
 			old_zone = NULL;
 		}
 		result = configure_rpz_zone(
-			view, zone_element, recursive_only_default,
+			view, zone_element, zoneit, recursive_only_default,
 			add_soa_default, ttl_default, minupdateinterval_default,
 			old_zone, old_rpz_okp);
 		if (result != ISC_R_SUCCESS) {
 			if (pview_must_detach) {
 				dns_view_detach(&pview);
 			}
-			return result;
+			goto out;
 		}
+
+		hasmorezone = cfgmgr_nextit(zoneit) == ISC_R_SUCCESS;
 	}
 
 	/*
@@ -2371,6 +2363,9 @@ configure_rpz(dns_view_t *view, dns_view_t *pview, const cfg_obj_t *rpz_obj,
 		dns_view_detach(&pview);
 	}
 
+ out:
+	cfgmgr_putit(&zoneit);
+	cfgmgr_putit(&rpzit);
 	return ISC_R_SUCCESS;
 }
 
@@ -3807,10 +3802,10 @@ static const char *const response_synonyms[] = { "response", NULL };
  */
 static isc_result_t
 configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
-	       cfg_obj_t *vconfig, named_cachelist_t *cachelist,
-	       dns_kasplist_t *kasplist, dns_keystorelist_t *keystores,
-	       const cfg_obj_t *bindkeys, isc_mem_t *mctx,
-	       cfg_aclconfctx_t *actx, bool need_hints) {
+	       cfg_obj_t *vconfig, cfgmgr_it_t *viewit,
+	       named_cachelist_t *cachelist, dns_kasplist_t *kasplist,
+	       dns_keystorelist_t *keystores, const cfg_obj_t *bindkeys,
+	       isc_mem_t *mctx, cfg_aclconfctx_t *actx, bool need_hints) {
 	const cfg_obj_t *maps[4];
 	const cfg_obj_t *cfgmaps[3];
 	const cfg_obj_t *optionmaps[3];
@@ -3820,8 +3815,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	const cfg_obj_t *forwarders;
 	const cfg_obj_t *alternates;
 	const cfg_obj_t *zonelist;
-	const cfg_obj_t *dlzlist;
-	const cfg_obj_t *dlz;
 	const cfg_obj_t *prefetch_trigger;
 	const cfg_obj_t *prefetch_eligible;
 	unsigned int dlzargc;
@@ -3871,9 +3864,24 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	dns_ntatable_t *ntatable = NULL;
 	const char *qminmode = NULL;
 	dns_adb_t *adb = NULL;
+	cfgmgr_it_t *optionsit = NULL;
 
 	REQUIRE(DNS_VIEW_VALID(view));
 
+	/*
+	 * The view implicitly have `options` properties. So when
+	 * viewit is non null, we can access all options-related
+	 * settings from the view directly from itview. But if viewit
+	 * is not provided (so we're in the default created view) then
+	 * we need an iterator to the top-level options clause.
+	 */
+	if (viewit == NULL) {
+		REQUIRE(cfgmgr_getit(NULL, &optionsit, "options") ==
+			ISC_R_SUCCESS);
+	} else {
+		optionsit = viewit;
+	}
+
 	if (config != NULL) {
 		(void)cfg_map_get(config, "options", &options);
 	}
@@ -3905,7 +3913,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	/*
 	 * Set the view's port number for outgoing queries.
 	 */
-	CHECKM(named_config_getport(config, "port", &port), "port");
+	CHECKM(named_config_getport("port", &port), "port");
 	dns_view_setdstport(view, port);
 
 	/*
@@ -3916,7 +3924,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	if (view->rdclass == dns_rdataclass_in && need_hints &&
 	    named_config_get(maps, "response-policy", &obj) == ISC_R_SUCCESS)
 	{
-		CHECK(configure_rpz(view, NULL, obj, &old_rpz_ok));
+		CHECK(configure_rpz(optionsit, view, NULL, obj, &old_rpz_ok));
 		rpz_configured = true;
 	}
 
@@ -3996,44 +4004,34 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	/*
 	 * Create Dynamically Loadable Zone driver.
 	 */
-	dlzlist = NULL;
-	if (voptions != NULL) {
-		(void)cfg_map_get(voptions, "dlz", &dlzlist);
-	} else {
-		(void)cfg_map_get(config, "dlz", &dlzlist);
-	}
+	cfgmgr_it_t *dlzit = NULL;
+	bool hasdlz = cfgmgr_getit(viewit, &dlzit, "dlz") == ISC_R_SUCCESS;
+	while (hasdlz) {
+		isc_result_t getresult;
+		char database[1024];
 
-	for (element = cfg_list_first(dlzlist); element != NULL;
-	     element = cfg_list_next(element))
-	{
-		dlz = cfg_listelt_value(element);
+		REQUIRE(dlzit);
 
-		obj = NULL;
-		(void)cfg_map_get(dlz, "database", &obj);
-		if (obj != NULL) {
+		getresult = cfgmgr_getstring(dlzit, "database", database, 1024);
+		if (getresult == ISC_R_SUCCESS) {
 			dns_dlzdb_t *dlzdb = NULL;
-			const cfg_obj_t *name, *search = NULL;
-			char *s = isc_mem_strdup(mctx, cfg_obj_asstring(obj));
+			char name[512];
+			bool search = false;
 
-			if (s == NULL) {
-				result = ISC_R_NOMEMORY;
-				goto cleanup;
-			}
-
-			result = isc_commandline_strtoargv(mctx, s, &dlzargc,
-							   &dlzargv, 0);
+			result = isc_commandline_strtoargv(
+				mctx, database, &dlzargc, &dlzargv, 0);
 			if (result != ISC_R_SUCCESS) {
-				isc_mem_free(mctx, s);
 				goto cleanup;
 			}
 
-			name = cfg_map_getname(dlz);
-			result = dns_dlzcreate(mctx, cfg_obj_asstring(name),
-					       dlzargv[0], dlzargc, dlzargv,
-					       &dlzdb);
-			isc_mem_free(mctx, s);
+			getresult = cfgmgr_getstring(dlzit, "name", name, 512);
+			INSIST(getresult == ISC_R_SUCCESS);
+
+			result = dns_dlzcreate(mctx, name, dlzargv[0], dlzargc,
+					       dlzargv, &dlzdb);
 			isc_mem_cput(mctx, dlzargv, dlzargc, sizeof(*dlzargv));
 			if (result != ISC_R_SUCCESS) {
+				cfgmgr_putit(&dlzit);
 				goto cleanup;
 			}
 
@@ -4043,12 +4041,13 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 			 * method now.  If not searchable, we'll take
 			 * care of it when we process the zone statement.
 			 */
-			(void)cfg_map_get(dlz, "search", &search);
-			if (search == NULL || cfg_obj_asboolean(search)) {
+			getresult = cfgmgr_getbool(dlzit, "search", &search);
+			if (getresult != ISC_R_SUCCESS || search) {
 				dlzdb->search = true;
 				result = dns_dlzconfigure(
 					view, dlzdb, dlzconfigure_callback);
 				if (result != ISC_R_SUCCESS) {
+					cfgmgr_putit(&dlzit);
 					goto cleanup;
 				}
 				ISC_LIST_APPEND(view->dlz_searched, dlzdb,
@@ -4059,7 +4058,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 						link);
 			}
 		}
+
+		hasdlz = cfgmgr_nextit(dlzit) == ISC_R_SUCCESS;
 	}
+	cfgmgr_putit(&dlzit);
 
 	/*
 	 * Obtain configuration parameters that affect the decision of whether
@@ -4511,8 +4513,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	/*
 	 * Resolver.
 	 */
-	CHECK(get_view_querysource_dispatch(maps, AF_INET, &dispatch4));
-	CHECK(get_view_querysource_dispatch(maps, AF_INET6, &dispatch6));
+	CHECK(get_view_querysource_dispatch(optionsit, AF_INET, &dispatch4));
+	CHECK(get_view_querysource_dispatch(optionsit, AF_INET6, &dispatch6));
 	if (dispatch4 == NULL && dispatch6 == NULL) {
 		UNEXPECTED_ERROR("unable to obtain either an IPv4 or"
 				 " an IPv6 dispatch");
@@ -4751,7 +4753,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	alternates = NULL;
 	(void)named_config_get(maps, "dual-stack-servers", &alternates);
 	if (alternates != NULL) {
-		CHECK(configure_alternates(config, view, alternates));
+		CHECK(configure_alternates(view, alternates));
 	}
 
 	/*
@@ -5788,8 +5790,8 @@ cleanup:
 				 * because we are reverting the same operation
 				 * done previously in the "correct" order.
 				 */
-				result2 = configure_rpz(pview, view, obj,
-							&old_rpz_ok);
+				result2 = configure_rpz(optionsit, pview, view,
+							obj, &old_rpz_ok);
 				if (result2 != ISC_R_SUCCESS) {
 					isc_log_write(NAMED_LOGCATEGORY_GENERAL,
 						      NAMED_LOGMODULE_SERVER,
@@ -5892,6 +5894,10 @@ cleanup:
 		dns_dyndb_destroyctx(&dctx);
 	}
 
+	if (viewit == NULL) {
+		cfgmgr_putit(&optionsit);
+	}
+
 	return result;
 }
 
@@ -5911,8 +5917,7 @@ configure_hints(dns_view_t *view, const char *filename) {
 }
 
 static isc_result_t
-configure_alternates(const cfg_obj_t *config, dns_view_t *view,
-		     const cfg_obj_t *alternates) {
+configure_alternates(dns_view_t *view, const cfg_obj_t *alternates) {
 	const cfg_obj_t *portobj;
 	const cfg_obj_t *addresses;
 	const cfg_listelt_t *element;
@@ -5922,7 +5927,7 @@ configure_alternates(const cfg_obj_t *config, dns_view_t *view,
 	/*
 	 * Determine which port to send requests to.
 	 */
-	CHECKM(named_config_getport(config, "port", &port), "port");
+	CHECKM(named_config_getport("port", &port), "port");
 
 	if (alternates != NULL) {
 		portobj = cfg_tuple_get(alternates, "port");
@@ -6043,8 +6048,8 @@ configure_forward(const cfg_obj_t *config, dns_view_t *view,
 	/*
 	 * Determine which port to send forwarded requests to.
 	 */
-	CHECKM(named_config_getport(config, "port", &port), "port");
-	CHECKM(named_config_getport(config, "tls-port", &tls_port), "tls-port");
+	CHECKM(named_config_getport("port", &port), "port");
+	CHECKM(named_config_getport("tls-port", &tls_port), "tls-port");
 
 	if (forwarders != NULL) {
 		portobj = cfg_tuple_get(forwarders, "port");
@@ -6167,19 +6172,19 @@ cleanup:
 }
 
 static isc_result_t
-get_viewinfo(const cfg_obj_t *vconfig, const char **namep,
-	     dns_rdataclass_t *classp) {
+get_viewinfo(const cfgmgr_it_t *viewit, const cfg_obj_t *vconfig, char *name,
+	     size_t namelen, dns_rdataclass_t *classp) {
 	isc_result_t result = ISC_R_SUCCESS;
-	const char *viewname;
 	dns_rdataclass_t viewclass;
 
-	REQUIRE(namep != NULL && *namep == NULL);
+	REQUIRE(name != NULL);
 	REQUIRE(classp != NULL);
 
 	if (vconfig != NULL) {
 		const cfg_obj_t *classobj = NULL;
 
-		viewname = cfg_obj_asstring(cfg_tuple_get(vconfig, "name"));
+		REQUIRE(cfgmgr_getstring(viewit, "name", name, namelen) ==
+			ISC_R_SUCCESS);
 		classobj = cfg_tuple_get(vconfig, "class");
 		CHECK(named_config_getclass(classobj, dns_rdataclass_in,
 					    &viewclass));
@@ -6187,15 +6192,14 @@ get_viewinfo(const cfg_obj_t *vconfig, const char **namep,
 			isc_log_write(NAMED_LOGCATEGORY_GENERAL,
 				      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 				      "view '%s': class must not be meta",
-				      viewname);
+				      name);
 			CHECK(ISC_R_FAILURE);
 		}
 	} else {
-		viewname = "_default";
+		strncpy(name, "_default", namelen);
 		viewclass = dns_rdataclass_in;
 	}
 
-	*namep = viewname;
 	*classp = viewclass;
 
 cleanup:
@@ -6208,14 +6212,15 @@ cleanup:
  * If 'vconfig' is NULL, attach to the default view.
  */
 static isc_result_t
-find_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist,
-	  dns_view_t **viewp) {
+find_view(const cfgmgr_it_t *viewit, const cfg_obj_t *vconfig,
+	  dns_viewlist_t *viewlist, dns_view_t **viewp) {
 	isc_result_t result;
-	const char *viewname = NULL;
+	char viewname[512];
 	dns_rdataclass_t viewclass;
 	dns_view_t *view = NULL;
 
-	result = get_viewinfo(vconfig, &viewname, &viewclass);
+	memset(viewname, 0, 512);
+	result = get_viewinfo(viewit, vconfig, viewname, 512, &viewclass);
 	if (result != ISC_R_SUCCESS) {
 		return result;
 	}
@@ -6237,14 +6242,15 @@ find_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist,
  * The view created is attached to '*viewp'.
  */
 static isc_result_t
-create_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist,
-	    dns_view_t **viewp) {
+create_view(const cfgmgr_it_t *viewit, const cfg_obj_t *vconfig,
+	    dns_viewlist_t *viewlist, dns_view_t **viewp) {
 	isc_result_t result;
-	const char *viewname = NULL;
+	char viewname[512];
 	dns_rdataclass_t viewclass;
 	dns_view_t *view = NULL;
 
-	result = get_viewinfo(vconfig, &viewname, &viewclass);
+	memset(viewname, 0, 512);
+	result = get_viewinfo(viewit, vconfig, viewname, 512, &viewclass);
 	if (result != ISC_R_SUCCESS) {
 		return result;
 	}
@@ -8060,6 +8066,8 @@ load_configuration(const char *filename, named_server_t *server,
 		goto cleanup_config;
 	}
 
+	cfgmgr_init(named_g_mctx, config, named_g_config);
+
 	/* Let's recreate the TLS context cache */
 	if (server->tlsctx_server_cache != NULL) {
 		isc_tlsctx_cache_detach(&server->tlsctx_server_cache);
@@ -8464,7 +8472,7 @@ load_configuration(const char *filename, named_server_t *server,
 	if (named_g_port != 0) {
 		listen_port = named_g_port;
 	} else {
-		result = named_config_getport(config, "port", &listen_port);
+		result = named_config_getport("port", &listen_port);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_v6portset;
 		}
@@ -8754,13 +8762,17 @@ load_configuration(const char *filename, named_server_t *server,
 	/*
 	 * Create the views.
 	 */
+	cfgmgr_it_t *viewit = NULL;
+	isc_result_t cfgmgrr = cfgmgr_getit(NULL, &viewit, "view");
 	for (element = cfg_list_first(views); element != NULL;
 	     element = cfg_list_next(element))
 	{
+		REQUIRE(cfgmgrr == ISC_R_SUCCESS);
+
 		cfg_obj_t *vconfig = cfg_listelt_value(element);
 		dns_view_t *view = NULL;
 
-		result = create_view(vconfig, &viewlist, &view);
+		result = create_view(viewit, vconfig, &viewlist, &view);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_viewlist;
 		}
@@ -8773,7 +8785,10 @@ load_configuration(const char *filename, named_server_t *server,
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_viewlist;
 		}
+
+		cfgmgrr = cfgmgr_nextit(viewit);
 	}
+	cfgmgr_putit(&viewit);
 
 	/*
 	 * If there were no explicit views then we do the default
@@ -8781,8 +8796,12 @@ load_configuration(const char *filename, named_server_t *server,
 	 */
 	if (views == NULL) {
 		dns_view_t *view = NULL;
+		cfgmgr_it_t *optionsit = NULL;
+		REQUIRE(cfgmgr_getit(NULL, &optionsit, "options") ==
+			ISC_R_SUCCESS);
 
-		result = create_view(NULL, &viewlist, &view);
+		result = create_view(optionsit, NULL, &viewlist, &view);
+		cfgmgr_putit(&optionsit);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_viewlist;
 		}
@@ -8802,29 +8821,40 @@ load_configuration(const char *filename, named_server_t *server,
 	 * views that have zones were already created at parsing
 	 * time, but views with no zones must be created here.
 	 */
+	bool hasmore = cfgmgr_getit(NULL, &viewit, "view") == ISC_R_SUCCESS;
+
 	for (element = cfg_list_first(views); element != NULL;
 	     element = cfg_list_next(element))
 	{
+		isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
+			      ISC_LOG_INFO, "element=%p\n", element);
+		REQUIRE(hasmore);
+
 		cfg_obj_t *vconfig = cfg_listelt_value(element);
 		dns_view_t *view = NULL;
 
 		view = NULL;
-		result = find_view(vconfig, &viewlist, &view);
+		result = find_view(viewit, vconfig, &viewlist, &view);
 		if (result != ISC_R_SUCCESS) {
+			cfgmgr_putit(&viewit);
 			goto cleanup_cachelist;
 		}
 
 		result = configure_view(view, &viewlist, config, vconfig,
-					&cachelist, &server->kasplist,
+					viewit, &cachelist, &server->kasplist,
 					&server->keystorelist, bindkeys,
 					named_g_mctx, named_g_aclconfctx, true);
 		if (result != ISC_R_SUCCESS) {
+			cfgmgr_putit(&viewit);
 			dns_view_detach(&view);
 			goto cleanup_cachelist;
 		}
 		dns_view_freeze(view);
 		dns_view_detach(&view);
+
+		hasmore = cfgmgr_nextit(viewit) == ISC_R_SUCCESS;
 	}
+	cfgmgr_putit(&viewit);
 
 	/*
 	 * Make sure we have a default view if and only if there
@@ -8832,14 +8862,17 @@ load_configuration(const char *filename, named_server_t *server,
 	 */
 	if (views == NULL) {
 		dns_view_t *view = NULL;
-		result = find_view(NULL, &viewlist, &view);
+
+		result = find_view(NULL, NULL, &viewlist, &view);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_cachelist;
 		}
-		result = configure_view(view, &viewlist, config, NULL,
+
+		result = configure_view(view, &viewlist, config, NULL, NULL,
 					&cachelist, &server->kasplist,
 					&server->keystorelist, bindkeys,
 					named_g_mctx, named_g_aclconfctx, true);
+
 		if (result != ISC_R_SUCCESS) {
 			dns_view_detach(&view);
 			goto cleanup_cachelist;
@@ -8851,31 +8884,41 @@ load_configuration(const char *filename, named_server_t *server,
 	/*
 	 * Create (or recreate) the built-in views.
 	 */
+	isc_result_t r = cfgmgr_getit(NULL, &viewit, "builtinview");
+	hasmore = r == ISC_R_SUCCESS;
 	builtin_views = NULL;
 	RUNTIME_CHECK(cfg_map_get(named_g_config, "view", &builtin_views) ==
 		      ISC_R_SUCCESS);
 	for (element = cfg_list_first(builtin_views); element != NULL;
 	     element = cfg_list_next(element))
 	{
+		REQUIRE(viewit);
+		REQUIRE(hasmore);
+
 		cfg_obj_t *vconfig = cfg_listelt_value(element);
 		dns_view_t *view = NULL;
 
-		result = create_view(vconfig, &builtin_viewlist, &view);
+		result = create_view(viewit, vconfig, &builtin_viewlist, &view);
 		if (result != ISC_R_SUCCESS) {
+			cfgmgr_putit(&viewit);
 			goto cleanup_cachelist;
 		}
 
 		result = configure_view(
-			view, &viewlist, config, vconfig, &cachelist,
+			view, &viewlist, config, vconfig, viewit, &cachelist,
 			&server->kasplist, &server->keystorelist, bindkeys,
 			named_g_mctx, named_g_aclconfctx, false);
 		if (result != ISC_R_SUCCESS) {
+			cfgmgr_putit(&viewit);
 			dns_view_detach(&view);
 			goto cleanup_cachelist;
 		}
 		dns_view_freeze(view);
 		dns_view_detach(&view);
+
+		hasmore = cfgmgr_nextit(viewit) == ISC_R_SUCCESS;
 	}
+	cfgmgr_putit(&viewit);
 
 	/* Now combine the two viewlists into one */
 	ISC_LIST_APPENDLIST(viewlist, builtin_viewlist, link);
@@ -9654,6 +9697,8 @@ shutdown_server(void *arg) {
 		cfg_aclconfctx_detach(&named_g_aclconfctx);
 	}
 
+	cfgmgr_destroy();
+
 	cfg_obj_destroy(named_g_parser, &named_g_config);
 	cfg_parser_destroy(&named_g_parser);
 	cfg_parser_destroy(&named_g_addparser);
@@ -10925,7 +10970,7 @@ listenelt_fromconfig(const cfg_obj_t *listener, const cfg_obj_t *config,
 				port = named_g_httpsport;
 			} else {
 				result = named_config_getport(
-					config, "https-port", &port);
+					"https-port", &port);
 				if (result != ISC_R_SUCCESS) {
 					return result;
 				}
@@ -10935,7 +10980,7 @@ listenelt_fromconfig(const cfg_obj_t *listener, const cfg_obj_t *config,
 				port = named_g_httpport;
 			} else {
 				result = named_config_getport(
-					config, "http-port", &port);
+					"http-port", &port);
 				if (result != ISC_R_SUCCESS) {
 					return result;
 				}
@@ -10945,7 +10990,7 @@ listenelt_fromconfig(const cfg_obj_t *listener, const cfg_obj_t *config,
 				port = named_g_tlsport;
 			} else {
 				result = named_config_getport(
-					config, "tls-port", &port);
+					"tls-port", &port);
 				if (result != ISC_R_SUCCESS) {
 					return result;
 				}
@@ -10954,8 +10999,7 @@ listenelt_fromconfig(const cfg_obj_t *listener, const cfg_obj_t *config,
 			if (named_g_port != 0) {
 				port = named_g_port;
 			} else {
-				result = named_config_getport(config, "port",
-							      &port);
+				result = named_config_getport("port", &port);
 				if (result != ISC_R_SUCCESS) {
 					return result;
 				}

lib/isccfg/Makefile.am

@@ -7,6 +7,7 @@ libisccfg_la_HEADERS =			\
 	include/isccfg/aclconf.h	\
 	include/isccfg/cfg.h		\
 	include/isccfg/check.h		\
+	include/isccfg/cfgmgr.h		\
 	include/isccfg/duration.h	\
 	include/isccfg/grammar.h	\
 	include/isccfg/kaspconf.h	\
@@ -16,6 +17,7 @@ libisccfg_la_SOURCES =			\
 	$(libisccfg_la_HEADERS)		\
 	aclconf.c			\
 	check.c				\
+	cfgmgr.c 			\
 	duration.c			\
 	kaspconf.c			\
 	namedconf.c			\

lib/isccfg/cfgmgr.README

@@ -0,0 +1,176 @@
+# Introduction
+
+This is a (relatively) high level explanation of the cfgmgr
+experiments and API proposal. The goal of this experiment is to
+propose (as a first phase) a new API that would be a replacement to
+cfg_* usages in the whole code-base.
+
+The internal data structures and implementations of
+lib/isccfg/cfgmgr.{c,h} relies on existing cfg_* structs and
+APIs. This enables to define a new configuration manager interface w/o
+having to worry about how things should be ideally implemented
+internally (yet).
+
+Currently the life-cycle of cfgmgr is tight to
+server.c:load_configuration (and use cfg_obj_t objects provided by it
+after configuration files are parsed) but eventually it would be the
+life-cycle of the named process, so various parts of named could
+dynamically read configuration data without relying on a
+re-initialization process when the configuration is updated.
+
+Note that cfgmgr internal data currently rely on internal static
+variables. Let's keep this in mind if this is a problem at some
+point. (either for unit testing, design, flexibility reasons).
+
+Finding the right data model/where it lives/how long it lives would be
+the next step, once every cfg_* consumers use cfgmgr_* instead (so
+cfg_* things could be internal to cfgmgr until cfgmgr define its own
+model, then cfg_* could go away). Well, that's the naive/ideal plan.
+
+
+
+# Concepts and API
+
+An important concept of this API is the "iterator". Basically, it can
+be seen as a "handle" to a configuration clause. For instance, to
+access `query-source` property of the `options` clause, the caller
+first gets an iterator to the `options` clause (through the
+`cfgmgr_getit` API), then it asks for a sockaddr value of the
+`query-source` property. The name is "iterator" because some (lot of)
+clauses are repeatable, and getting the next iterator (through the
+provided `cfgmgr_nextit` API) gives a sense that next clause is from
+the same type (it might not be clearer is the name was "nexthandle" or
+"nextclause"). Furthermore, there can be multiple sub-clauses: as an
+example, `server`, `dlz` etc. can be inside a `view` as well as
+top-level. The same iterator concept enable accessing those nested
+clauses. It is possible because the `cfgmgr_getit` API takes an
+optional pre-existing (context) iterator when trying to access a
+clause. This would then return an iterator to a clause from within a
+clause. This is a fundamental object of this API as it should
+(hopefully) give enough flexibility to have a very generic mechanism
+to navigate though the configuration and (hopefully) should remain
+valid even with a different named configuration syntax/format.
+
+The term "clause" used just above has a bit flexible definition. For
+instance, the response-policy option can have multiple zone
+"clauses". As it's not yet 100% clear what kind of configurations
+entities are iterables, the term "iterator" feels right for now, as
+generic element to move forward in a serie of elements.
+
+If getting an iterator seems cumbersome, another approach could be to
+provide a "fully-qualified key", i.e. "options.query-source" but even
+though there would be a need for something enabling to navigate in
+nested structures, so the iterator concept seems the most viable at
+this time of this experiment.
+
+One last note about the iterators: those are an opaque type for the
+consumers. The type `cfgmgr_it_t` can't be de-referenced outside
+cfgmgr. Its actual implementation `iterator_t` is internal to cfgmgr,
+but it's obviously not relevant from an API standpoint.
+
+Currently focus is on "reading" data to figure out the right API, so
+the cfgmgr_set* functions are not implemented, nor the
+cfgmgr_{commit,revert,begintransaction} functions. But there is two
+things to keep in mind: (1) the update mechanism will be transactional
+(hence the "commit", "revert", "begintransaction" names) as this would
+enable updates w/o introducing inconsistencies in the configuration,
+(assuming at some point some named components read configuration data
+directly instead of caching them), and (2) when `cfgmgr_commit` would
+be called, all iterators should be released. This is to avoid having
+iterator pointing to potentially dangling data, depending how the
+model works/how it would be updated or if an iterator points to a
+clause what would be removed. Technically speaking, it means that
+cfgmgr would ultimately have its own memory context to allocate
+iterators, so it could check all iterators has been released at commit
+time.
+
+When using `cfgmgr_get*` API to read configurations values, it is
+important to note that everything should be copied (i.e. no pointer
+should be given to the caller). The main reason is because doing so
+would open the door wide open to consumer code holding a pointer to a
+configuration data which would get invalidated after a configuration
+update. More-or-less the same reason that all iterators must be
+released when `cfgmgr_commit` is called (even if not yet implemented).
+
+
+
+# Performance consideration
+
+Performances aren't too much a concern (for now) because even though
+cfmgr likely to internally adds lot of indirections, lookups and
+string comparison to go through the cfg_* nested structures, this (1)
+occurs (so far) only during server initialization time then should
+(hopefully) be negligible and (2) the goal is to move to a different
+implementation which would be way quicker and (3) parts of code which
+actually needs to be _really_ fast (i.e. if a configuration value is
+read millions times per second) then it's reasonable enough this part
+of code cache the configuration value internally anyway. (In which
+case it will be needed to consider a callback/notification API which
+would tell such part of code to refresh its config cache if
+configuration has changed)
+
+
+
+# Alernatives and related works
+
+- jemalloc uses a namespace with dot string API,
+  i.e. `foo.bar.<i>.gee` to access `gee` value at the i-th element of
+  the `bar` list inside `foo`. While it's easy to have a small buffer
+  around to build such string (in case of iterations), BIND view
+  configuration re-use lot of top-level options block parameters, and
+  all of those are read in the same configuration function with an
+  optional pointer confiugration to a view. Using such API in this
+  context would require manually building something like
+  `view.<name-or-i>.query-source` _or_
+  `options.query-source`. depending if we're in a view or at
+  top-level. I'm worry it gets quickly messy and confusing w/o major
+  rework/refactoring of the configuration code. I didn't tried that
+  though, so maybe I'm wrong and it's perfectly fine. In term of
+  implementation it does store data in a tree (see
+  https://github.com/jemalloc/jemalloc/blob/46690c9ec036cede074476caa05ecd6fe954bd23/src/ctl.c#L1022)
+  in memory where each node can have a function to read/write its
+  value. See
+  https://github.com/jemalloc/jemalloc/blob/46690c9ec036cede074476caa05ecd6fe954bd23/src/ctl.c#L2103
+  and definition
+  https://github.com/jemalloc/jemalloc/blob/46690c9ec036cede074476caa05ecd6fe954bd23/src/ctl.c#L66C11-L66C28. Note
+  lookup is done by a various set of functions.But I understand it
+  either all boils down to a tree visit and string comparison
+  https://github.com/jemalloc/jemalloc/blob/46690c9ec036cede074476caa05ecd6fe954bd23/src/ctl.c#L1588
+  or tree visit and indexed access with nodes
+  https://github.com/jemalloc/jemalloc/blob/46690c9ec036cede074476caa05ecd6fe954bd23/src/ctl.c#L1725
+  which I guess would be faster, but it's still a tree visit, do lot
+  of dereferencing pointers etc. I believe there is no notification
+  mechanism when a setting is updated, but because each node
+  (configuration value) implement its own setter function, the
+  function can directly call/do whatever needed to flush/force
+  reconfiguration of various parts of the code depending on this
+  setting).
+
+- sysrepo is existing library implementing YANG. This has several
+  advantages as it could be useful later to have a standarized
+  configuration file format without having to write a specific parser,
+  and it already implement a configuration store and APIs. I plan to
+  make some experiments
+
+- Knot seems to use something relatively close to the current cfgmgr
+  experiment where it provide set of API to access a config value
+  within a configuration context (Which I understand can be use to dig
+  into nested clauses) See
+  https://gitlab.nic.cz/knot/knot-dns/-/blob/master/src/knot/conf/conf.h?ref_type=heads#L769
+  and
+  https://gitlab.nic.cz/knot/knot-dns/-/blob/master/src/knot/server/server.c#L246
+  as example.
+
+- For the record, I also considered a totally different approach that
+  would be basically a struct holding each possible configuration
+  values (so there would be nested structs, list, etc.). That would
+  have the advantage of being super straightfoward to read into
+  (simply get a pointer and we're done) but (1) if at some point we
+  parse a grammar definition file to automatically generate the
+  parser/the code that build up the configuration, this would require
+  manual changes then (2) whatever memory model we're using to update
+  the config could be easilly abused by mistake (i.e. holding a
+  pointer to a nested struct while we flip the configuration and free
+  the old one) and (3) it's likely difficult to integrate with high
+  level API (HTTP for BIND management for instance) where every
+  propoerty would need custom code to be marshalled/accessed.etc.

lib/isccfg/cfgmgr.c

@@ -0,0 +1,546 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <isc/buffer.h>
+
+#include <isccfg/cfgmgr.h>
+#include <isccfg/grammar.h>
+#include <isccfg/namedconf.h>
+
+#define CFGLEN 6
+typedef struct iterator iterator_t;
+struct iterator {
+	/*
+	 * Some clauses (at least dlz) gets they name through
+	 * cfg_map_getname. But it's really tricky to figure this out
+	 * at lookup time (in getobj). For now, let's consider this as
+	 * a specific case, but let's try to find a better solution
+	 * later (i.e. include the name in the cfgmap?)
+	 */
+	const char *name;
+
+	/*
+	 * Various maps where a configuration for a given clause are
+	 * provided. THe latest one is the default. NULL terminated
+	 * array.
+	 */
+	cfg_obj_t const *configs[CFGLEN];
+
+	/*
+	 * If the clause is repeatable (i.e view, etc) this point to
+	 * the current clause and the function to update the current
+	 * iterator to the following one
+	 */
+	const cfg_listelt_t *cur;
+	isc_result_t (*walk)(const iterator_t *, iterator_t *);
+};
+
+static isc_mem_t *mctx = NULL;
+static const cfg_obj_t *defaults = NULL;
+static const cfg_obj_t *config = NULL;
+
+/*
+ * Mostly copied from parser.c:cfg_tuple_get, at the difference that
+ * the parser version must return a value - but it might not exists in
+ * this context
+ */
+static isc_result_t
+getobjfromtuple(const cfg_obj_t *tuple, const char *name,
+		const cfg_obj_t **obj) {
+	unsigned int i;
+	const cfg_tuplefielddef_t *fields;
+	const cfg_tuplefielddef_t *f;
+
+	REQUIRE(obj != NULL && *obj == NULL);
+	REQUIRE(tuple != NULL && tuple->type->rep == &cfg_rep_tuple);
+	REQUIRE(name != NULL);
+
+	fields = tuple->type->of;
+	for (f = fields, i = 0; f->name != NULL; f++, i++) {
+		if (strcmp(f->name, name) == 0) {
+			*obj = tuple->value.tuple[i];
+			return ISC_R_SUCCESS;
+		}
+	}
+
+	INSIST(*obj == NULL);
+	return ISC_R_NOTFOUND;
+}
+
+static isc_result_t
+getobj(const cfgmgr_it_t *it, const char *name, const cfg_obj_t **obj) {
+	isc_result_t result = ISC_R_NOTFOUND;
+	const iterator_t *itp;
+
+	REQUIRE(it);
+	REQUIRE(obj && *obj == NULL);
+
+	itp = it;
+	for (size_t i = 0; itp->configs[i] != NULL; i++) {
+		if (cfg_obj_ismap(itp->configs[i])) {
+			result = cfg_map_get(itp->configs[i], name, obj);
+		} else if (cfg_obj_istuple(itp->configs[i])) {
+			result = getobjfromtuple(itp->configs[i], name, obj);
+		} else {
+			UNREACHABLE();
+		}
+
+		if (result == ISC_R_SUCCESS) {
+			break;
+		}
+		INSIST(*obj == NULL);
+	}
+
+	return result;
+}
+
+static isc_result_t
+setoptionsconfig(const iterator_t *ctxit, iterator_t *it) {
+	isc_result_t result;
+	const cfg_obj_t *obj = NULL;
+	size_t i = 0;
+
+	REQUIRE(ctxit == NULL);
+	REQUIRE(it);
+
+	result = cfg_map_get(config, "options", &obj);
+	REQUIRE(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	obj = NULL;
+	result = cfg_map_get(defaults, "options", &obj);
+	REQUIRE(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	obj = NULL;
+	REQUIRE(getobj(it, "query-source", &obj) == ISC_R_SUCCESS);
+	obj = NULL;
+	REQUIRE(getobj(it, "query-source-v6", &obj) == ISC_R_SUCCESS);
+
+	INSIST(it->cur == NULL);
+      	return result;
+}
+
+static isc_result_t
+setviewconfig(const iterator_t *ctxit, iterator_t *it) {
+	isc_result_t result;
+	const cfg_obj_t *obj = NULL;
+	size_t i = 0;
+
+	REQUIRE(ctxit == NULL);
+	REQUIRE(it);
+
+	if (it->cur == NULL) {
+		result = cfg_map_get(config, "view", &obj);
+		if (result != ISC_R_SUCCESS) {
+			goto out;
+		}
+
+		INSIST(cfg_obj_islist(obj));
+		it->cur = cfg_list_first(obj);
+	} else {
+		it->cur = cfg_list_next(it->cur);
+		if (!it->cur) {
+			result = ISC_R_NOMORE;
+			goto out;
+		}
+	}
+
+	obj = cfg_listelt_value(it->cur);
+
+	INSIST(obj);
+
+	/*
+	 * name and view class are in the tuple object, so this makes
+	 * them accessible. The options are at the top-level of the
+	 * view, so we put them directly in the config object.
+	 */
+	it->configs[i++] = obj;
+
+	it->configs[i] = cfg_tuple_get(obj, "options");
+	INSIST(it->configs[i++]);
+
+	obj = NULL;
+	result = cfg_map_get(config, "options", &obj);
+	INSIST(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	obj = NULL;
+	result = cfg_map_get(defaults, "options", &obj);
+	INSIST(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	obj = NULL;
+	INSIST(getobj(it, "query-source", &obj) == ISC_R_SUCCESS);
+	obj = NULL;
+	INSIST(getobj(it, "query-source-v6", &obj) == ISC_R_SUCCESS);
+
+	it->walk = setviewconfig;
+out:
+	return result;
+}
+
+static isc_result_t
+setbuiltinviewconfig(const iterator_t *ctxit, iterator_t *it) {
+	isc_result_t result;
+	const cfg_obj_t *obj = NULL;
+	size_t i = 0;
+
+	REQUIRE(ctxit == NULL);
+	REQUIRE(it);
+
+	if (it->cur == NULL) {
+		result = cfg_map_get(defaults, "view", &obj);
+		INSIST(result == ISC_R_SUCCESS);
+		INSIST(cfg_obj_islist(obj));
+		it->cur = cfg_list_first(obj);
+	} else {
+		it->cur = cfg_list_next(it->cur);
+		if (!it->cur) {
+			result = ISC_R_NOMORE;
+			goto out;
+		}
+	}
+
+	obj = cfg_listelt_value(it->cur);
+	INSIST(obj);
+
+	/*
+	 * name and view class are in the tuple object, so this makes
+	 * them accessible. The options are at the top-level of the
+	 * view, so we put them directly in the config object.
+	 */
+	it->configs[i++] = obj;
+
+	it->configs[i] = cfg_tuple_get(obj, "options");
+	INSIST(it->configs[i++]);
+
+	obj = NULL;
+	result = cfg_map_get(config, "options", &obj);
+	INSIST(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	obj = NULL;
+	result = cfg_map_get(defaults, "options", &obj);
+	INSIST(result == ISC_R_SUCCESS);
+	it->configs[i++] = obj;
+
+	it->walk = setbuiltinviewconfig;
+out:
+	return result;
+}
+
+static isc_result_t
+setresponsepolicyconfig(const iterator_t *ctxit, iterator_t *it) {
+	const cfg_obj_t *obj = NULL;
+	isc_result_t result = ISC_R_SUCCESS;
+
+	REQUIRE(it);
+	REQUIRE(ctxit);
+
+	result = getobj(ctxit, "response-policy", &obj);
+	if (result != ISC_R_SUCCESS) {
+		goto out;
+	}
+
+	INSIST(obj);
+	it->configs[0] = obj;
+		
+ out:
+	INSIST(it->cur == NULL);
+	return result;
+}
+
+static isc_result_t
+setzonelistconfig(const iterator_t *ctxit, iterator_t *it) {
+	const cfg_obj_t *obj = NULL;
+	isc_result_t result = ISC_R_SUCCESS;
+
+	REQUIRE(it);
+
+	if (ctxit == NULL) {
+		/*
+		 * Not for first element of the zone list, moving forward
+		 */
+		if (it->cur != NULL) {
+			it->cur = cfg_list_next(it->cur);
+		}
+
+		if (it->cur == NULL) {
+			result = ISC_R_NOMORE;
+			goto out;
+		}
+	} else {
+		/*
+		 * "zone list" is an internal list of a clause, it has to be
+		 * in a clause context.
+		 */
+		result = getobj(ctxit, "zone list", &obj);
+		if (result != ISC_R_SUCCESS) {
+			goto out;
+		}
+		it->cur = cfg_list_first(obj);
+	}
+	it->configs[0] = cfg_listelt_value(it->cur);
+	it->walk = setzonelistconfig;
+
+ out:
+	return result;
+}
+
+static isc_result_t
+setdlzconfig(const iterator_t *ctxit, iterator_t *it) {
+	const cfg_obj_t *obj = NULL;
+	isc_result_t result = ISC_R_SUCCESS;
+	REQUIRE(it);
+
+	if (it->cur == NULL) {
+		if (ctxit != NULL) {
+			result = getobj(ctxit, "dlz", &obj);
+		} else {
+			result = cfg_map_get(config, "dlz", &obj);
+		}
+
+		if (result != ISC_R_SUCCESS) {
+			goto out;
+		}
+		INSIST(cfg_obj_islist(obj));
+		it->cur = cfg_list_first(obj);
+	} else {
+		it->cur = cfg_list_next(it->cur);
+		if (it->cur == NULL) {
+			result = ISC_R_NOMORE;
+			goto out;
+		}
+	}
+
+	INSIST(result == ISC_R_SUCCESS);
+
+	obj = cfg_listelt_value(it->cur);
+	INSIST(obj);
+	it->configs[0] = obj;
+
+	obj = cfg_map_getname(obj);
+	INSIST(obj);
+	it->name = cfg_obj_asstring(obj);
+
+	it->walk = setdlzconfig;
+out:
+	return result;
+}
+
+#define CLAUSE(rhs) if (strcasecmp(name, rhs) == 0)
+
+static isc_result_t
+setconfig(const cfgmgr_it_t *ctxit, iterator_t *it, const char *name) {
+	isc_result_t result = ISC_R_NOTFOUND;
+
+	CLAUSE("options") {
+		result = setoptionsconfig(ctxit, it);
+		goto out;
+	}
+
+	CLAUSE("view") {
+		result = setviewconfig(ctxit, it);
+		goto out;
+	}
+
+	CLAUSE("builtinview") {
+		result = setbuiltinviewconfig(ctxit, it);
+		goto out;
+	}
+
+	CLAUSE("dlz") {
+		result = setdlzconfig(ctxit, it);
+		goto out;
+	}
+
+	CLAUSE("response-policy") {
+		result = setresponsepolicyconfig(ctxit, it);
+		goto out;
+	}
+
+	CLAUSE("zone list") {
+		result = setzonelistconfig(ctxit, it);
+		goto out;
+	}
+
+	UNREACHABLE();
+
+out:
+	return result;
+}
+
+isc_result_t
+cfgmgr_init(isc_mem_t *mctx_, const cfg_obj_t *config_,
+	    const cfg_obj_t *defaults_) {
+	mctx = mctx_;
+	config = config_;
+	defaults = defaults_;
+
+	INSIST(mctx);
+	INSIST(config);
+	INSIST(defaults);
+
+	return ISC_R_SUCCESS;
+}
+
+void
+cfgmgr_destroy() {}
+
+isc_result_t
+cfgmgr_getit(const cfgmgr_it_t *ctxit, cfgmgr_it_t **it, const char *name) {
+	isc_result_t result = ISC_R_SUCCESS;
+	iterator_t *itp = NULL;
+
+	REQUIRE(it != NULL && *it == NULL);
+
+	itp = isc_mem_cget(mctx, 1, sizeof(*itp));
+	result = setconfig(ctxit, itp, name);
+
+	if (result != ISC_R_SUCCESS) {
+		isc_mem_put(mctx, itp, sizeof(*itp));
+	} else {
+		*it = itp;
+	}
+
+	return result;
+}
+
+void
+cfgmgr_putit(cfgmgr_it_t **it) {
+	REQUIRE(it != NULL);
+
+	if (*it) {
+		iterator_t *itp = *it;
+		isc_mem_put(mctx, itp, sizeof(*itp));
+		*it = NULL;
+	}
+}
+
+isc_result_t
+cfgmgr_nextit(cfgmgr_it_t *it) {
+	isc_result_t result = ISC_R_NOMORE;
+	iterator_t *itp = NULL;
+
+	REQUIRE(it);
+	itp = (iterator_t *)it;
+
+	itp->name = NULL;
+	memset(itp->configs, 0, sizeof(cfg_obj_t *) * CFGLEN);
+	if (itp->cur) {
+		REQUIRE(itp->walk);
+		result = itp->walk(NULL, itp);
+	}
+
+	return result;
+}
+
+#define EXTRACTVALUE_OP(type, op)					\
+	do {								\
+		isc_result_t result = ISC_R_NOTFOUND;			\
+		const cfg_obj_t *obj = NULL;				\
+		REQUIRE(it);						\
+		REQUIRE(name);						\
+		REQUIRE(value);						\
+	        result = getobj(it, name, &obj);			\
+		if (result == ISC_R_SUCCESS) {				\
+			if (cfg_obj_is##type(obj)) {			\
+				op;					\
+			} else {					\
+				result = ISC_R_NOTFOUND;		\
+			}						\
+		}							\
+		return result;						\
+	} while (0)
+
+
+#define EXTRACTSCALARVALUE(type)					\
+	EXTRACTVALUE_OP(type, *value = cfg_obj_as##type(obj))
+
+#define EXTRACTPTRVALUE(type)						\
+	EXTRACTVALUE_OP(type, memcpy(value, cfg_obj_as##type(obj),	\
+				     sizeof(*value)))
+
+isc_result_t
+cfgmgr_getbool(const cfgmgr_it_t *it, const char *name, bool *value) {
+	EXTRACTSCALARVALUE(boolean);
+}
+
+isc_result_t
+cfgmgr_getuint32(const cfgmgr_it_t *it, const char *name, uint32_t *value) {
+	EXTRACTSCALARVALUE(uint32);
+}
+
+isc_result_t
+cfgmgr_getduration(const cfgmgr_it_t *it, const char *name, uint32_t *value) {
+	EXTRACTSCALARVALUE(duration);
+}
+
+isc_result_t
+cfgmgr_getstring(const cfgmgr_it_t *it, const char *name, char *buf,
+		 size_t len) {
+	isc_result_t result = ISC_R_NOTFOUND;
+	const cfg_obj_t *obj = NULL;
+	const char *str = NULL;
+
+	REQUIRE(it);
+	REQUIRE(name);
+
+	result = getobj(it, name, &obj);
+	if (result == ISC_R_SUCCESS) {
+		if (cfg_obj_isstring(obj)) {
+			str = cfg_obj_asstring(obj);
+		} else {
+			result = ISC_R_NOTFOUND;
+		}
+	}
+
+	if (str == NULL && strcasecmp(name, "name") == 0) {
+		str = ((iterator_t *)it)->name;
+	}
+
+	if (str != NULL) {
+		if (strlen(str) + 1 > len) {
+			result = ISC_R_NOSPACE;
+		} else {
+			(void)strcpy(buf, str);
+			result = ISC_R_SUCCESS;
+		}
+	}
+
+	return result;
+}
+
+isc_result_t
+cfgmgr_getsockaddr(const cfgmgr_it_t *it, const char *name,
+		   isc_sockaddr_t *value) {
+	EXTRACTPTRVALUE(sockaddr);
+}
+
+isc_result_t
+cfgmgr_getnone(const cfgmgr_it_t *it, const char *name) {
+	const cfg_obj_t *obj = NULL;
+	isc_result_t result;
+
+	REQUIRE(name);
+
+	result = getobj(it, name, &obj);
+	if (result == ISC_R_SUCCESS) {
+		if (!cfg_obj_isvoid(obj)) {
+			result = ISC_R_NOTFOUND;
+		}
+	}
+
+	return result;
+}

lib/isccfg/include/isccfg/cfgmgr.h

@@ -0,0 +1,146 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#pragma once
+
+#include <isc/sockaddr.h>
+
+#include <isccfg/cfg.h>
+
+typedef void cfgmgr_it_t;
+/*%<
+ * An handle to a cfgmgr internal iterator enabling caller code to
+ * go through repeatable clauses. The type is opaque thus no consumer
+ * code should try to read/cast/do anything with it except passing
+ * around cfgmgr functions.
+ */
+
+isc_result_t
+cfgmgr_init(isc_mem_t *mctx, const cfg_obj_t *config,
+	    const cfg_obj_t *defaults);
+
+void
+cfgmgr_destroy(void);
+
+isc_result_t
+cfgmgr_getit(const cfgmgr_it_t *ctxit, cfgmgr_it_t **it, const char *name);
+/*%<
+ * Allocate an iterator pointing the first element of the clause "name"
+ * and set its address to the consumer-provided "it" pointer. If
+ * "name" is not found, "it" pointer is set to NULL and ISC_R_NOTFOUND
+ * is returned.
+ */
+
+void
+cfgmgr_putit(cfgmgr_it_t **it);
+/*%<
+ * Free the iterator and set "it" pointer to NULL.
+ */
+
+isc_result_t
+cfgmgr_nextit(cfgmgr_it_t *it);
+/*%<
+ * Moves the iterator to the next clause. Returns ISC_R_NOMORE when
+ * there is no other clause of the same type.
+ */
+
+isc_result_t
+cfgmgr_getbool(const cfgmgr_it_t *it, const char *name, bool *value);
+/*%<
+ * Read a boolean value from the "name" option in the clause
+ * references by "it" and copy it into "value". If the value is not
+ * found from the configuration, "value" isn't modified. Same applies
+ * for all the cfgmgr_get* API below.
+ */
+
+isc_result_t
+cfgmgr_getuint32(const cfgmgr_it_t *it, const char *name, uint32_t *value);
+
+/*
+ * Even if getduration returns an uint32_t, it's probably a good idea
+ * to keep the distinction: because internally the duration can be
+ * stored in hours, minutes, etc. And we might not want to convert it
+ * into the second inside cfgmgr in case we need to dump the
+ * configuration: we might want to keep the original unit to avoid
+ * confusion from users. To be discussed.
+ */
+isc_result_t
+cfgmgr_getduration(const cfgmgr_it_t *it, const char *name, uint32_t *value);
+
+isc_result_t
+cfgmgr_getstring(const cfgmgr_it_t *it, const char *name, char *buf,
+		 size_t len);
+
+isc_result_t
+cfgmgr_getsockaddr(const cfgmgr_it_t *it, const char *name,
+		   isc_sockaddr_t *addr);
+
+isc_result_t
+cfgmgr_getnone(const cfgmgr_it_t *it, const char *name);
+/*%<
+ * At the difference of the others cfgmgr_get* APIs, this one
+ * doesn't have any value. The return values tells if `none` is
+ * present (ISC_R_SUCCESS) or not.
+ */
+
+void
+cfgmgr_begintransaction(void);
+/*%<
+ * Begin a transaction: this enable to modify configuration options
+ * with cfgmgr_set* and cfgmgr_reset functions. When an option is
+ * modified, the old value is still returned from cfgmgr_get* until
+ * cfgmgr_commit() is called. Can't be called under a transaction.
+ */
+
+void
+cfgmgr_commit(void);
+/*%<
+ * Must be called under a transaction. Atomically apply the new
+ * configuration, which is the previous configuration with all
+ * modifications done using cfgmgr_set* or cfgmgr_reset
+ * functions. Caller code must take care of calling cfgmgr_putit for
+ * all allocated iterators before calling this function.
+ */
+
+void
+cfgmgr_revert(void);
+/*%<
+ * Must be called under a transaction. Undo all the changes made by
+ * cfgmgr_set* or cfgmgr_reset calls and close the transaction.
+ */
+
+void
+cfgmgr_reset(const cfgmgr_it_t *it, const char *name);
+/*%<
+ * Must be called under a transaction. Set the option at "name" to
+ * its default value in the clause referenced by "it". It no default
+ * exists, it is removed.
+ *
+ * If "it" is not NULL but "name" is, remove the configuration clause
+ * referenced by "it". If "it" and "name" are both NULL, this reset
+ * the whole named configuration to its default value.
+ *
+ * TODO: To be figured out if the cases there "it" is NULL are
+ * realistic.
+ */
+
+void
+cfgmgr_setuint64(const cfgmgr_it_t *it, const char *name,
+		 const uint64_t value);
+
+void
+cfgmgr_setsockaddr(const cfgmgr_it_t *it, const char *name,
+		   const isc_sockaddr_t *addr);
+
+void
+cfgmgr_setnone(const cfgmgr_it_t *it, const char *name);