mgmtd: simplify early config build removing unused code

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

mgmtd/mgmt_be_adapter.c

@@ -602,8 +602,8 @@ static void mgmt_be_adapter_process_msg(uint8_t version, uint8_t *data,
 	mgmtd__be_message__free_unpacked(be_msg, NULL);
 }
 
-static void mgmt_be_iter_and_get_cfg(struct mgmt_ds_ctx *ds_ctx, char *xpath,
+static void mgmt_be_iter_and_get_cfg(struct mgmt_ds_ctx *ds_ctx,
-				     struct lyd_node *node,
+				     const char *xpath, struct lyd_node *node,
 				     struct nb_node *nb_node, void *ctx)
 {
 	struct mgmt_be_client_subscr_info subscr_info;
@@ -784,19 +784,24 @@ int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
 				  struct mgmt_ds_ctx *ds_ctx,
 				  struct nb_config_cbs **cfg_chgs)
 {
-	char base_xpath[] = "/";
 	struct mgmt_be_get_adapter_config_params parms;
 
 	assert(cfg_chgs);
 
+	/*
+	 * TODO: we should be consider making this an assertable condition and
+	 * guaranteeing it be true when this function is called. B/c what is
+	 * going to happen if there are some changes being sent, and we don't
+	 * gather a new snapshot, what new changes that came after the previous
+	 * snapshot will then be lost?
+	 */
 	if (RB_EMPTY(nb_config_cbs, &adapter->cfg_chgs)) {
 		parms.adapter = adapter;
 		parms.cfg_chgs = &adapter->cfg_chgs;
 		parms.seq = 0;
 
-		mgmt_ds_iter_data(ds_ctx, base_xpath,
+		mgmt_ds_iter_data(ds_ctx, "", mgmt_be_iter_and_get_cfg,
-				  mgmt_be_iter_and_get_cfg, (void *)&parms,
+				  (void *)&parms);
-				  false);
 	}
 
 	*cfg_chgs = &adapter->cfg_chgs;

mgmtd/mgmt_ds.c

@@ -312,36 +312,28 @@ struct nb_config *mgmt_ds_get_nb_config(struct mgmt_ds_ctx *ds_ctx)
 }
 
 static int mgmt_walk_ds_nodes(
-	struct mgmt_ds_ctx *ds_ctx, char *base_xpath,
+	struct mgmt_ds_ctx *ds_ctx, const char *base_xpath,
 	struct lyd_node *base_dnode,
-	void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx, char *xpath,
+	void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx,
-				     struct lyd_node *node,
+				     const char *xpath, struct lyd_node *node,
 				     struct nb_node *nb_node, void *ctx),
-	void *ctx, char *xpaths[], int *num_nodes, bool childs_as_well,
+	void *ctx)
-	bool alloc_xp_copy)
 {
-	uint32_t indx;
+	/* this is 1k per recursion... */
-	char *xpath, *xpath_buf, *iter_xp;
+	char xpath[MGMTD_MAX_XPATH_LEN];
-	int ret, num_left = 0, num_found = 0;
 	struct lyd_node *dnode;
 	struct nb_node *nbnode;
-	bool alloc_xp = false;
+	int ret = 0;
 
-	if (xpaths)
+	assert(mgmt_ds_node_iter_fn);
-		assert(num_nodes);
 
-	if (num_nodes && !*num_nodes)
+	MGMTD_DS_DBG(" -- START: base xpath: '%s'", base_xpath);
-		return 0;
-
-	if (num_nodes) {
-		num_left = *num_nodes;
-		MGMTD_DS_DBG(" -- START: num_left:%d", num_left);
-		*num_nodes = 0;
-	}
-
-	MGMTD_DS_DBG(" -- START: Base: '%s'", base_xpath);
 
 	if (!base_dnode)
+		/*
+		 * This function only returns the first node of a possible set
+		 * of matches issuing a warning if more than 1 matches
+		 */
 		base_dnode = yang_dnode_get(
 			ds_ctx->config_ds ? ds_ctx->root.cfg_root->dnode
 					   : ds_ctx->root.dnode_root,
@@ -349,111 +341,42 @@ static int mgmt_walk_ds_nodes(
 	if (!base_dnode)
 		return -1;
 
-	if (mgmt_ds_node_iter_fn) {
+	MGMTD_DS_DBG("           search base schema: '%s'",
-		/*
+		     lysc_path(base_dnode->schema, LYSC_PATH_LOG, xpath,
-		 * In case the caller is interested in getting a copy
+			       sizeof(xpath)));
-		 * of the xpath for themselves (by setting
-		 * 'alloc_xp_copy' to 'true') we make a copy for the
-		 * caller and pass it. Else we pass the original xpath
-		 * buffer.
-		 *
-		 * NOTE: In such case caller will have to take care of
-		 * the copy later.
-		 */
-		iter_xp = alloc_xp_copy ? strdup(base_xpath) : base_xpath;
 
 	nbnode = (struct nb_node *)base_dnode->schema->priv;
-		(*mgmt_ds_node_iter_fn)(ds_ctx, iter_xp, base_dnode, nbnode,
+	(*mgmt_ds_node_iter_fn)(ds_ctx, base_xpath, base_dnode, nbnode, ctx);
-					ctx);
-	}
-
-	if (num_nodes) {
-		(*num_nodes)++;
-		num_left--;
-	}
 
 	/*
-	 * If the base_xpath points to a leaf node, or we don't need to
+	 * If the base_xpath points to a leaf node we can skip the tree walk.
-	 * visit any children we can skip the tree walk.
 	 */
-	if (!childs_as_well || base_dnode->schema->nodetype & LYD_NODE_TERM)
+	if (base_dnode->schema->nodetype & LYD_NODE_TERM)
 		return 0;
 
-	indx = 0;
+	/*
+	 * at this point the xpath matched this container node (or some parent
+	 * and we're wildcard descending now) so by walking it's children we
+	 * continue to change the meaning of an xpath regex to rather be a
+	 * prefix matching path
+	 */
+
 	LY_LIST_FOR (lyd_child(base_dnode), dnode) {
 		assert(dnode->schema && dnode->schema->priv);
 
-		xpath = NULL;
+		(void)lyd_path(dnode, LYD_PATH_STD, xpath, sizeof(xpath));
-		if (xpaths) {
-			if (!xpaths[*num_nodes]) {
-				alloc_xp = true;
-				xpaths[*num_nodes] =
-					(char *)calloc(1, MGMTD_MAX_XPATH_LEN);
-			}
-			xpath = lyd_path(dnode, LYD_PATH_STD,
-					 xpaths[*num_nodes],
-					 MGMTD_MAX_XPATH_LEN);
-		} else {
-			alloc_xp = true;
-			xpath_buf = (char *)calloc(1, MGMTD_MAX_XPATH_LEN);
-			(void) lyd_path(dnode, LYD_PATH_STD, xpath_buf,
-					 MGMTD_MAX_XPATH_LEN);
-			xpath = xpath_buf;
-		}
 
-		assert(xpath);
+		MGMTD_DS_DBG(" -- Child xpath: %s", xpath);
-		MGMTD_DS_DBG(" -- XPATH: %s", xpath);
-
-		if (num_nodes)
-			num_found = num_left;
 
 		ret = mgmt_walk_ds_nodes(ds_ctx, xpath, dnode,
-					 mgmt_ds_node_iter_fn, ctx,
+					 mgmt_ds_node_iter_fn, ctx);
-					 xpaths ? &xpaths[*num_nodes] : NULL,
-					 num_nodes ? &num_found : NULL,
-					 childs_as_well, alloc_xp_copy);
-
-		if (num_nodes) {
-			num_left -= num_found;
-			(*num_nodes) += num_found;
-		}
-
-		if (alloc_xp)
-			free(xpath);
-
 		if (ret != 0)
 			break;
-
-		indx++;
 	}
 
+	MGMTD_DS_DBG(" -- END: base xpath: '%s'", base_xpath);
 
-	if (num_nodes) {
+	return ret;
-		MGMTD_DS_DBG(" -- END: *num_nodes:%d, num_left:%d", *num_nodes,
-			     num_left);
-	}
-
-	return 0;
-}
-
-int mgmt_ds_lookup_data_nodes(struct mgmt_ds_ctx *ds_ctx, const char *xpath,
-			      char *dxpaths[], int *num_nodes,
-			      bool get_childs_as_well, bool alloc_xp_copy)
-{
-	char base_xpath[MGMTD_MAX_XPATH_LEN];
-
-	if (!ds_ctx || !num_nodes)
-		return -1;
-
-	if (xpath[0] == '.' && xpath[1] == '/')
-		xpath += 2;
-
-	strlcpy(base_xpath, xpath, sizeof(base_xpath));
-	mgmt_remove_trailing_separator(base_xpath, '/');
-
-	return (mgmt_walk_ds_nodes(ds_ctx, base_xpath, NULL, NULL, NULL,
-				   dxpaths, num_nodes, get_childs_as_well,
-				   alloc_xp_copy));
 }
 
 struct lyd_node *mgmt_ds_find_data_node_by_xpath(struct mgmt_ds_ctx *ds_ctx,
@@ -534,13 +457,13 @@ int mgmt_ds_load_config_from_file(struct mgmt_ds_ctx *dst,
 	return 0;
 }
 
-int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, char *base_xpath,
+int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, const char *base_xpath,
 		      void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx,
-						   char *xpath,
+						   const char *xpath,
 						   struct lyd_node *node,
 						   struct nb_node *nb_node,
 						   void *ctx),
-		      void *ctx, bool alloc_xp_copy)
+		      void *ctx)
 {
 	int ret = 0;
 	char xpath[MGMTD_MAX_XPATH_LEN];
@@ -550,14 +473,19 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, char *base_xpath,
 	if (!ds_ctx)
 		return -1;
 
-	mgmt_remove_trailing_separator(base_xpath, '/');
-
 	strlcpy(xpath, base_xpath, sizeof(xpath));
+	mgmt_remove_trailing_separator(xpath, '/');
+
+	/*
+	 * mgmt_ds_iter_data is the only user of mgmt_walk_ds_nodes other than
+	 * mgmt_walk_ds_nodes itself, so we can modify the API if we would like.
+	 * Oper-state should be kept in mind though for the prefix walk
+	 */
 
 	MGMTD_DS_DBG(" -- START DS walk for DSid: %d", ds_ctx->ds_id);
 
 	/* If the base_xpath is empty then crawl the sibblings */
-	if (xpath[0] == '\0') {
+	if (xpath[0] == 0) {
 		base_dnode = ds_ctx->config_ds ? ds_ctx->root.cfg_root->dnode
 						: ds_ctx->root.dnode_root;
 
@@ -569,14 +497,12 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, char *base_xpath,
 			base_dnode = base_dnode->prev;
 
 		LY_LIST_FOR (base_dnode, node) {
-			ret = mgmt_walk_ds_nodes(
+			ret = mgmt_walk_ds_nodes(ds_ctx, xpath, node,
-				ds_ctx, xpath, node, mgmt_ds_node_iter_fn,
+						 mgmt_ds_node_iter_fn, ctx);
-				ctx, NULL, NULL, true, alloc_xp_copy);
 		}
 	} else
 		ret = mgmt_walk_ds_nodes(ds_ctx, xpath, base_dnode,
-					 mgmt_ds_node_iter_fn, ctx, NULL, NULL,
+					 mgmt_ds_node_iter_fn, ctx);
-					 true, alloc_xp_copy);
 
 	return ret;
 }

mgmtd/mgmt_ds.h

@@ -217,39 +217,6 @@ extern int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx,
  */
 extern struct nb_config *mgmt_ds_get_nb_config(struct mgmt_ds_ctx *ds_ctx);
 
-/*
- * Lookup YANG data nodes.
- *
- * ds_ctx
- *    Datastore context.
- *
- * xpath
- *    YANG base xpath.
- *
- * dxpaths
- *    Out param - array of YANG data xpaths.
- *
- * num_nodes
- *    In-out param - number of YANG data xpaths.
- *    Note - Caller should init this to the size of the array
- *    provided in dxpaths.
- *    On return this will have the actual number of xpaths
- *    being returned.
- *
- * get_childs_as_well
- *    TRUE if child nodes needs to be fetched as well, FALSE otherwise.
- *
- * alloc_xp_copy
- *    TRUE if the caller is interested in getting a copy of the xpath.
- *
- * Returns:
- *    0 on success, -1 on failure.
- */
-extern int mgmt_ds_lookup_data_nodes(struct mgmt_ds_ctx *ds_ctx,
-				     const char *xpath, char *dxpaths[],
-				     int *num_nodes, bool get_childs_as_well,
-				     bool alloc_xp_copy);
-
 /*
  * Find YANG data node given a datastore handle YANG xpath.
  */
@@ -281,18 +248,15 @@ extern int mgmt_ds_delete_data_nodes(struct mgmt_ds_ctx *ds_ctx,
  *    be passed to the iterator function provided in
  *    'iter_fn'.
  *
- * alloc_xp_copy
- *    TRUE if the caller is interested in getting a copy of the xpath.
- *
  * Returns:
  *    0 on success, -1 on failure.
  */
 extern int mgmt_ds_iter_data(
-	struct mgmt_ds_ctx *ds_ctx, char *base_xpath,
+	struct mgmt_ds_ctx *ds_ctx, const char *base_xpath,
-	void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx, char *xpath,
+	void (*mgmt_ds_node_iter_fn)(struct mgmt_ds_ctx *ds_ctx,
-				     struct lyd_node *node,
+				     const char *xpath, struct lyd_node *node,
 				     struct nb_node *nb_node, void *ctx),
-	void *ctx, bool alloc_xp_copy);
+	void *ctx);
 
 /*
  * Load config to datastore from a file.

mgmtd/mgmt_txn.c

@@ -1743,7 +1743,7 @@ static void mgmt_txn_send_getcfg_reply_data(struct mgmt_txn_req *txn_req,
 }
 
 static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx,
-						  char *xpath,
+						 const char *xpath,
 						 struct lyd_node *node,
 						 struct nb_node *nb_node,
 						 void *ctx)
@@ -1756,10 +1756,10 @@ static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx,
 
 	txn_req = (struct mgmt_txn_req *)ctx;
 	if (!txn_req)
-		goto mgmtd_ignore_get_cfg_reply_data;
+		return;
 
 	if (!(node->schema->nodetype & LYD_NODE_TERM))
-		goto mgmtd_ignore_get_cfg_reply_data;
+		return;
 
 	assert(txn_req->req_event == MGMTD_TXN_PROC_GETCFG
 	       || txn_req->req_event == MGMTD_TXN_PROC_GETDATA);
@@ -1771,7 +1771,7 @@ static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx,
 	data_value = &get_reply->reply_value[get_reply->num_reply];
 
 	mgmt_yang_data_init(data);
-	data->xpath = xpath;
+	data->xpath = strdup(xpath);
 	mgmt_yang_data_value_init(data_value);
 	data_value->value_case = MGMTD__YANG_DATA_VALUE__VALUE_ENCODED_STR_VAL;
 	data_value->encoded_str_val = (char *)lyd_get_value(node);
@@ -1784,12 +1784,6 @@ static void mgmt_txn_iter_and_send_get_cfg_reply(struct mgmt_ds_ctx *ds_ctx,
 
 	if (get_reply->num_reply == MGMTD_MAX_NUM_DATA_REPLY_IN_BATCH)
 		mgmt_txn_send_getcfg_reply_data(txn_req, get_req);
-
-	return;
-
-mgmtd_ignore_get_cfg_reply_data:
-	if (xpath)
-		free(xpath);
 }
 
 static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn,
@@ -1824,10 +1818,14 @@ static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn,
 		MGMTD_TXN_DBG("Trying to get all data under '%s'",
 			       get_data->xpaths[indx]);
 		mgmt_init_get_data_reply(get_reply);
+		/*
+		 * mgmt_ds_iter_data works on path prefixes, but the user may
+		 * want to also use an xpath regexp we need to add this
+		 * functionality.
+		 */
 		if (mgmt_ds_iter_data(get_data->ds_ctx, get_data->xpaths[indx],
 				      mgmt_txn_iter_and_send_get_cfg_reply,
-				      (void *)txn_req, true)
+				      (void *)txn_req) == -1) {
-		    == -1) {
 			MGMTD_TXN_DBG("Invalid Xpath '%s",
 				       get_data->xpaths[indx]);
 			mgmt_fe_send_get_cfg_reply(