lib: fix iteration over schema nodes of a single YANG module

The only safe way to iterate over all schema nodes of a given YANG
module is by iterating over all schema nodes of all YANG modules
and filter out the nodes that belong to other modules.

The original yang_snodes_iterate_module() code did the following:
1 - Iterate over all top-level schema nodes of the given module;
2 - Iterate over all augmentations of the given module.

While that iteration strategy is more efficient, it does't handle
well more complex YANG hierarchies containing nested augmentations
or self-augmenting modules. Any iteration that isn't done on the
resolved YANG data hierarchy is fragile and prone to errors.

Fixes regression introduced by commit 8a923b48513316b where the
gen_northbound_callbacks tool was generating duplicate callbacks
for certain modules.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

4 files changed, 32 insertions, 19 deletions

diff --git a/lib/northbound.c b/lib/northbound.c
index c99f993ea..e79ccc7dd 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -152,8 +152,10 @@ static int nb_node_del_cb(const struct lys_node *snode, void *arg)
 	struct nb_node *nb_node;
 
 	nb_node = snode->priv;
-	lys_set_private(snode, NULL);
-	XFREE(MTYPE_NB_NODE, nb_node);
+	if (nb_node) {
+		lys_set_private(snode, NULL);
+		XFREE(MTYPE_NB_NODE, nb_node);
+	}
 
 	return YANG_ITER_CONTINUE;
 }
@@ -273,8 +275,10 @@ static int nb_node_validate(const struct lys_node *snode, void *arg)
 	unsigned int *errors = arg;
 
 	/* Validate callbacks and priority. */
-	*errors += nb_node_validate_cbs(nb_node);
-	*errors += nb_node_validate_priority(nb_node);
+	if (nb_node) {
+		*errors += nb_node_validate_cbs(nb_node);
+		*errors += nb_node_validate_priority(nb_node);
+	}
 
 	return YANG_ITER_CONTINUE;
 }
diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c
index c1cb0fc11..a53e90d55 100644
--- a/lib/northbound_confd.c
+++ b/lib/northbound_confd.c
@@ -550,6 +550,9 @@ static int frr_confd_init_cdb(void)
 				continue;
 
 			nb_node = snode->priv;
+			if (!nb_node)
+				continue;
+
 			DEBUGD(&nb_dbg_client_confd, "%s: subscribing to '%s'",
 			       __func__, nb_node->xpath);
 
@@ -1189,7 +1192,7 @@ static int frr_confd_subscribe_state(const struct lys_node *snode, void *arg)
 	struct nb_node *nb_node = snode->priv;
 	struct confd_data_cbs *data_cbs = arg;
 
-	if (!CHECK_FLAG(snode->flags, LYS_CONFIG_R))
+	if (!nb_node || !CHECK_FLAG(snode->flags, LYS_CONFIG_R))
 		return YANG_ITER_CONTINUE;
 	/* We only need to subscribe to the root of the state subtrees. */
 	if (snode->parent && CHECK_FLAG(snode->parent->flags, LYS_CONFIG_R))
@@ -1393,7 +1396,8 @@ static int frr_confd_calculate_snode_hash(const struct lys_node *snode,
 {
 	struct nb_node *nb_node = snode->priv;
 
-	nb_node->confd_hash = confd_str2hash(snode->name);
+	if (nb_node)
+		nb_node->confd_hash = confd_str2hash(snode->name);
 
 	return YANG_ITER_CONTINUE;
 }
diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index 3cd310c5a..e9aac3ef1 100644
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -575,6 +575,8 @@ static int frr_sr_subscribe_state(const struct lys_node *snode, void *arg)
 		return YANG_ITER_CONTINUE;
 
 	nb_node = snode->priv;
+	if (!nb_node)
+		return YANG_ITER_CONTINUE;
 
 	DEBUGD(&nb_dbg_client_sysrepo, "sysrepo: providing data to '%s'",
 	       nb_node->xpath);
@@ -599,6 +601,8 @@ static int frr_sr_subscribe_rpc(const struct lys_node *snode, void *arg)
 		return YANG_ITER_CONTINUE;
 
 	nb_node = snode->priv;
+	if (!nb_node)
+		return YANG_ITER_CONTINUE;
 
 	DEBUGD(&nb_dbg_client_sysrepo, "sysrepo: providing RPC to '%s'",
 	       nb_node->xpath);
diff --git a/lib/yang.c b/lib/yang.c
index 5bf7758e1..46012acf5 100644
--- a/lib/yang.c
+++ b/lib/yang.c
@@ -230,22 +230,23 @@ next:
 int yang_snodes_iterate_module(const struct lys_module *module,
 			       yang_iterate_cb cb, uint16_t flags, void *arg)
 {
-	struct lys_node *snode;
+	const struct lys_module *module_iter;
+	uint32_t idx = 0;
 	int ret = YANG_ITER_CONTINUE;
 
-	LY_TREE_FOR (module->data, snode) {
-		ret = yang_snodes_iterate_subtree(snode, module, cb, flags,
-						  arg);
-		if (ret == YANG_ITER_STOP)
-			return ret;
-	}
+	idx = ly_ctx_internal_modules_count(ly_native_ctx);
+	while ((module_iter = ly_ctx_get_module_iter(ly_native_ctx, &idx))) {
+		struct lys_node *snode;
 
-	for (uint8_t i = 0; i < module->augment_size; i++) {
-		ret = yang_snodes_iterate_subtree(
-			(const struct lys_node *)&module->augment[i], module,
-			cb, flags, arg);
-		if (ret == YANG_ITER_STOP)
-			return ret;
+		if (!module_iter->implemented)
+			continue;
+
+		LY_TREE_FOR (module_iter->data, snode) {
+			ret = yang_snodes_iterate_subtree(snode, module, cb,
+							  flags, arg);
+			if (ret == YANG_ITER_STOP)
+				return ret;
+		}
 	}
 
 	return ret;