lib: implement missing YANG choice/case statements.

Signed-off-by: Christian Hopps <chopps@labn.net>

lib/northbound_oper.c

@@ -69,8 +69,10 @@ struct nb_op_node_info {
  * @xpath: current xpath representing the node_info stack.
  * @xpath_orig: the original query string from the user
  * @node_infos: the container stack for the walk from root to current
- * @schema_path: the schema nodes for each node in the query string.
+ * @schema_path: the schema nodes along the path indicated by the query string.
- # @query_tokstr: the query string tokenized with NUL bytes.
+ *               this will include the choice and case nodes which are not
+ *               present in the query string.
+ * @query_tokstr: the query string tokenized with NUL bytes.
  * @query_tokens: the string pointers to each query token (node).
  * @non_specific_predicate: tracks if a query_token is non-specific predicate.
  * @walk_root_level: The topmost specific node, +1 is where we start walking.
@@ -204,6 +206,14 @@ static void ys_pop_inner(struct nb_op_yield_state *ys)
 	ys_trim_xpath(ys);
 }
 
+static void ys_free_inner(struct nb_op_yield_state *ys,
+			  struct nb_op_node_info *ni)
+{
+	if (!CHECK_FLAG(ni->schema->nodetype, LYS_CASE | LYS_CHOICE))
+		lyd_free_tree(&ni->inner->node);
+	ni->inner = NULL;
+}
+
 static void nb_op_get_keys(struct lyd_node_inner *list_node,
 			   struct yang_list_keys *keys)
 {
@@ -254,8 +264,7 @@ static bool __move_back_to_next(struct nb_op_yield_state *ys, int i)
 	 * The i'th node has been lost after a yield so trim it from the tree
 	 * now.
 	 */
-	lyd_free_tree(&ni->inner->node);
+	ys_free_inner(ys, ni);
-	ni->inner = NULL;
 	ni->list_entry = NULL;
 
 	/*
@@ -296,7 +305,7 @@ static void nb_op_resume_data_tree(struct nb_op_yield_state *ys)
 		ni = &ys->node_infos[i];
 		nn = ni->schema->priv;
 
-		if (CHECK_FLAG(ni->schema->nodetype, LYS_CONTAINER))
+		if (!CHECK_FLAG(ni->schema->nodetype, LYS_LIST))
 			continue;
 
 		assert(ni->list_entry != NULL ||
@@ -418,7 +427,8 @@ static enum nb_error nb_op_ys_finalize_node_info(struct nb_op_yield_state *ys,
 	/* Assert that we are walking the rightmost branch */
 	assert(!inner->parent || &inner->node == inner->parent->child->prev);
 
-	if (CHECK_FLAG(inner->schema->nodetype, LYS_CONTAINER)) {
+	if (CHECK_FLAG(inner->schema->nodetype,
+		       LYS_CASE | LYS_CHOICE | LYS_CONTAINER)) {
 		/* containers have only zero or one child on a branch of a tree */
 		inner = (struct lyd_node_inner *)inner->child;
 		assert(!inner || inner->prev == &inner->node);
@@ -502,9 +512,12 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys)
 			ret = NB_ERR_NOT_FOUND;
 		return ret;
 	}
-	while (node &&
+
+	/* Move up to the container if on a leaf currently. */
+	if (node &&
 	    !CHECK_FLAG(node->schema->nodetype, LYS_CONTAINER | LYS_LIST))
 		node = &node->parent->node;
+	assert(CHECK_FLAG(node->schema->nodetype, LYS_CONTAINER | LYS_LIST));
 	if (!node)
 		return NB_ERR_NOT_FOUND;
 
@@ -512,7 +525,6 @@ static enum nb_error nb_op_ys_init_node_infos(struct nb_op_yield_state *ys)
 	for (len = 1; inner->parent; len++)
 		inner = inner->parent;
 
-
 	darr_append_nz_mt(ys->node_infos, len, MTYPE_NB_NODE_INFOS);
 
 	/*
@@ -761,7 +773,7 @@ static const struct lysc_node *nb_op_sib_next(struct nb_op_yield_state *ys,
 	/*
 	 * If the node info stack is shorter than the schema path then we are
 	 * doign specific query still on the node from the schema path (should
-	 * match) so just return NULL.
+	 * match) so just return NULL (i.e., don't process siblings)
 	 */
 	if (darr_len(ys->schema_path) > darr_len(ys->node_infos))
 		return NULL;
@@ -826,7 +838,10 @@ static const struct lysc_node *nb_op_sib_first(struct nb_op_yield_state *ys,
 	 * base of the user query, return the next schema node from the query
 	 * string (schema_path).
 	 */
-	assert(darr_last(ys->node_infos) != NULL && darr_last(ys->node_infos)->schema == parent);
+	if (darr_last(ys->node_infos) != NULL &&
+	    !CHECK_FLAG(darr_last(ys->node_infos)->schema->nodetype,
+			LYS_CASE | LYS_CHOICE))
+		assert(darr_last(ys->node_infos)->schema == parent);
 	if (darr_lasti(ys->node_infos) < ys->query_base_level)
 		return ys->schema_path[darr_lasti(ys->node_infos) + 1];
 
@@ -937,28 +952,30 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
 		if (!sib) {
 			/*
 			 * We've reached the end of the siblings inside a
-			 * containing node; either a container or a specific
+			 * containing node; either a container, case, choice, or
-			 * list node entry.
+			 * a specific list node entry.
 			 *
-			 * We handle container node inline; however, for lists
+			 * We handle case/choice/container node inline; however,
-			 * we are only done with a specific entry and need to
+			 * for lists we are only done with a specific entry and
-			 * move to the next element on the list so we drop down
+			 * need to move to the next element on the list so we
-			 * into the switch for that case.
+			 * drop down into the switch for that case.
 			 */
 
 			/* Grab the containing node. */
 			sib = ni->schema;
 
-			if (sib->nodetype == LYS_CONTAINER) {
+			if (CHECK_FLAG(sib->nodetype,
+				       LYS_CASE | LYS_CHOICE | LYS_CONTAINER)) {
 				/* If we added an empty container node (no
 				 * children) and it's not a presence container
 				 * or it's not backed by the get_elem callback,
 				 * remove the node from the tree.
 				 */
-				if (!lyd_child(&ni->inner->node) &&
+				if (sib->nodetype == LYS_CONTAINER &&
+				    !lyd_child(&ni->inner->node) &&
 				    !nb_op_empty_container_ok(sib, ys->xpath,
 							      ni->list_entry))
-					lyd_free_tree(&ni->inner->node);
+					ys_free_inner(ys, ni);
 
 				/* If we have returned to our original walk base,
 				 * then we are done with the walk.
@@ -995,11 +1012,13 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
 			 */
 		}
 
-		/* TODO: old code checked for "first" here and skipped if set */
 		if (CHECK_FLAG(sib->nodetype,
 			       LYS_LEAF | LYS_LEAFLIST | LYS_CONTAINER))
 			xpath_child = nb_op_get_child_path(ys->xpath, sib,
 							   xpath_child);
+		else if (CHECK_FLAG(sib->nodetype, LYS_CASE | LYS_CHOICE))
+			darr_in_strdup(xpath_child, ys->xpath);
+
 		nn = sib->priv;
 
 		switch (sib->nodetype) {
@@ -1032,27 +1051,31 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
 				goto done;
 			sib = nb_op_sib_next(ys, sib);
 			continue;
+		case LYS_CASE:
+		case LYS_CHOICE:
 		case LYS_CONTAINER:
 			if (CHECK_FLAG(nn->flags, F_NB_NODE_CONFIG_ONLY)) {
 				sib = nb_op_sib_next(ys, sib);
 				continue;
 			}
 
-			node = NULL;
+			if (sib->nodetype != LYS_CONTAINER) {
-			err = lyd_new_inner(&ni->inner->node, sib->module,
+				/* Case/choice use parent inner. */
-					    sib->name, false, &node);
+				node = &ni->inner->node;
+			} else {
+				err = lyd_new_inner(&ni->inner->node,
+						    sib->module, sib->name,
+						    false, &node);
 				if (err) {
 					ret = NB_ERR_RESOURCE;
 					goto done;
 				}
+			}
 
-			/* push this container node on top of the stack */
+			/* push this choice/container node on top of the stack */
 			ni = darr_appendz(ys->node_infos);
 			ni->inner = (struct lyd_node_inner *)node;
-			ni->schema = node->schema;
+			ni->schema = sib;
-			ni->niters = 0;
-			ni->nents = 0;
-			ni->has_lookup_next = false;
 			ni->lookup_next_ok = ni[-1].lookup_next_ok;
 			ni->list_entry = ni[-1].list_entry;
 
@@ -1279,7 +1302,7 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
 			      lysc_is_key((*darr_last(ys->schema_path)))) &&
 			    /* is this at or below the base? */
 			    darr_ilen(ys->node_infos) <= ys->query_base_level)
-				lyd_free_tree(&ni->inner->node);
+				ys_free_inner(ys, ni);
 
 
 			if (!list_entry) {
@@ -1433,12 +1456,6 @@ static enum nb_error __walk(struct nb_op_yield_state *ys, bool is_resume)
 			sib = nb_op_sib_first(ys, sib);
 			continue;
 
-		case LYS_CHOICE:
-			/* Container type with no data */
-			/*FALLTHROUGH*/
-		case LYS_CASE:
-			/* Container type with no data */
-			/*FALLTHROUGH*/
 		default:
 			/*FALLTHROUGH*/
 		case LYS_ANYXML:
@@ -1534,8 +1551,7 @@ static void nb_op_trim_yield_state(struct nb_op_yield_state *ys)
 
 	DEBUGD(&nb_dbg_events, "NB oper-state: deleting tree at level %d", i);
 	__free_siblings(&ni->inner->node);
-	lyd_free_tree(&ni->inner->node);
+	ys_free_inner(ys, ni);
-	ni->inner = NULL;
 
 	while (--i > 0) {
 		DEBUGD(&nb_dbg_events,
@@ -1648,6 +1664,17 @@ static enum nb_error nb_op_ys_init_schema_path(struct nb_op_yield_state *ys,
 		int nlen = strlen(name);
 		int mnlen = 0;
 
+		/*
+		 * Technically the query_token for choice/case should probably be pointing at
+		 * the child (leaf) rather than the parent (container), however,
+		 * we only use these for processing list nodes so KISS.
+		 */
+		if (CHECK_FLAG(ys->schema_path[i]->nodetype,
+			       LYS_CASE | LYS_CHOICE)) {
+			ys->query_tokens[i] = ys->query_tokens[i - 1];
+			continue;
+		}
+
 		while (true) {
 			s2 = strstr(s, name);
 			if (!s2)

tests/lib/northbound/test_oper_data.c

@@ -200,6 +200,25 @@ static struct yang_data *frr_test_module_vrfs_vrf_routes_route_active_get_elem(
 	return NULL;
 }
 
+
+/*
+ * XPath: /frr-test-module:frr-test-module/c1value
+ */
+static struct yang_data *
+frr_test_module_c1value_get_elem(struct nb_cb_get_elem_args *args)
+{
+	return yang_data_new_uint8(args->xpath, 21);
+}
+
+/*
+ * XPath: /frr-test-module:frr-test-module/c2cont/c2value
+ */
+static struct yang_data *
+frr_test_module_c2cont_c2value_get_elem(struct nb_cb_get_elem_args *args)
+{
+	return yang_data_new_uint32(args->xpath, 0xAB010203);
+}
+
 /* clang-format off */
 const struct frr_yang_module_info frr_test_module_info = {
 	.name = "frr-test-module",
@@ -243,6 +262,14 @@ const struct frr_yang_module_info frr_test_module_info = {
 			.xpath = "/frr-test-module:frr-test-module/vrfs/vrf/routes/route/active",
 			.cbs.get_elem = frr_test_module_vrfs_vrf_routes_route_active_get_elem,
 		},
+		{
+			.xpath = "/frr-test-module:frr-test-module/c1value",
+			.cbs.get_elem = frr_test_module_c1value_get_elem,
+		},
+		{
+			.xpath = "/frr-test-module:frr-test-module/c2cont/c2value",
+			.cbs.get_elem = frr_test_module_c2cont_c2value_get_elem,
+		},
 		{
 			.xpath = NULL,
 		},

tests/lib/northbound/test_oper_data.refout

@@ -112,6 +112,10 @@ test# show yang operational-data /frr-test-module:frr-test-module
           }
         }
       ]
+    },
+    "c1value": 21,
+    "c2cont": {
+      "c2value": 2868969987
     }
   }
 }

yang/frr-test-module.yang

@@ -82,5 +82,23 @@ module frr-test-module {
         }
       }
     }
+    choice achoice {
+      description "a choice statement";
+      case case1 {
+        leaf c1value {
+          type uint8;
+          description "A uint8 value for case 1";
+        }
+      }
+      case case2 {
+        container c2cont {
+          description "case 2 container";
+          leaf c2value {
+            type uint32;
+            description "A uint32 value for case 2";
+          }
+        }
+      }
+    }
   }
 }