mgmtd: normalize argument order to copy(dst, src)

Having just completed a code audit during RCA, the fact that we have 2
different argument orders for the related datastore copying functions
was unnecessary and super confusing.

Fix this code-maintenance/comprehension mistake and move the newer mgmtd
copy routines to use the same arg order as the pre-existing underlying
northbound copy functions (i.e., use `copy(dst, src)`)

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

3 files changed, 23 insertions, 34 deletions

diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c
index dabae4afd1..e83980f97b 100644
--- a/mgmtd/mgmt_ds.c
+++ b/mgmtd/mgmt_ds.c
@@ -74,8 +74,7 @@ static int mgmt_ds_dump_in_memory(struct mgmt_ds_ctx *ds_ctx,
 	return 0;
 }
 
-static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
-					   struct mgmt_ds_ctx *dst)
+static int ds_copy(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src)
 {
 	if (!src || !dst)
 		return -1;
@@ -95,8 +94,7 @@ static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
 	return 0;
 }
 
-static int mgmt_ds_merge_src_with_dst_ds(struct mgmt_ds_ctx *src,
-					 struct mgmt_ds_ctx *dst)
+static int ds_merge(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src)
 {
 	int ret;
 
@@ -251,14 +249,13 @@ void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx)
 	ds_ctx->locked = 0;
 }
 
-int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx,
-		     struct mgmt_ds_ctx *dst_ds_ctx, bool updt_cmt_rec)
+int mgmt_ds_copy_dss(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src, bool updt_cmt_rec)
 {
-	if (mgmt_ds_replace_dst_with_src_ds(src_ds_ctx, dst_ds_ctx) != 0)
+	if (ds_copy(dst, src) != 0)
 		return -1;
 
-	if (updt_cmt_rec && dst_ds_ctx->ds_id == MGMTD_DS_RUNNING)
-		mgmt_history_new_record(dst_ds_ctx);
+	if (updt_cmt_rec && dst->ds_id == MGMTD_DS_RUNNING)
+		mgmt_history_new_record(dst);
 
 	return 0;
 }
@@ -416,9 +413,9 @@ int mgmt_ds_load_config_from_file(struct mgmt_ds_ctx *dst,
 	parsed.ds_id = dst->ds_id;
 
 	if (merge)
-		mgmt_ds_merge_src_with_dst_ds(&parsed, dst);
+		ds_merge(dst, &parsed);
 	else
-		mgmt_ds_replace_dst_with_src_ds(&parsed, dst);
+		ds_copy(dst, &parsed);
 
 	nb_config_free(parsed.root.cfg_root);
 
diff --git a/mgmtd/mgmt_ds.h b/mgmtd/mgmt_ds.h
index b8e77e330a..f7e1d7c5ee 100644
--- a/mgmtd/mgmt_ds.h
+++ b/mgmtd/mgmt_ds.h
@@ -196,21 +196,19 @@ extern void mgmt_ds_unlock(struct mgmt_ds_ctx *ds_ctx);
 /*
  * Copy from source to destination datastore.
  *
- * src_ds
- *    Source datastore handle (ds to be copied from).
- *
- * dst_ds
+ * dst
  *    Destination datastore handle (ds to be copied to).
  *
+ * src
+ *    Source datastore handle (ds to be copied from).
+ *
  * update_cmd_rec
  *    TRUE if need to update commit record, FALSE otherwise.
  *
  * Returns:
  *    0 on success, -1 on failure.
  */
-extern int mgmt_ds_copy_dss(struct mgmt_ds_ctx *src_ds_ctx,
-			    struct mgmt_ds_ctx *dst_ds_ctx,
-			    bool update_cmt_rec);
+extern int mgmt_ds_copy_dss(struct mgmt_ds_ctx *dst, struct mgmt_ds_ctx *src, bool update_cmt_rec);
 
 /*
  * Fetch northbound configuration for a given datastore context.
diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index 483dfab8e8..2b4734e971 100644
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -764,17 +764,15 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn,
 			 !txn->commit_cfg_req->req.commit_cfg.rollback);
 
 		/*
-		 * Successful commit: Merge Src DS into Dst DS if and only if
+		 * Successful commit: Copy Src DS to Dst DS if and only if
 		 * this was not a validate-only or abort request.
 		 */
 		if ((txn->session_id &&
 		     !txn->commit_cfg_req->req.commit_cfg.validate_only &&
 		     !txn->commit_cfg_req->req.commit_cfg.abort) ||
 		    txn->commit_cfg_req->req.commit_cfg.rollback) {
-			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-						 .src_ds_ctx,
-					 txn->commit_cfg_req->req.commit_cfg
-						 .dst_ds_ctx,
+			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx,
+					 txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
 					 create_cmt_info_rec);
 		}
 
@@ -783,22 +781,18 @@ static int mgmt_txn_send_commit_cfg_reply(struct mgmt_txn_ctx *txn,
 		 * request.
 		 */
 		if (txn->session_id && txn->commit_cfg_req->req.commit_cfg.abort)
-			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-						 .dst_ds_ctx,
-					 txn->commit_cfg_req->req.commit_cfg
-						 .src_ds_ctx,
-					 false);
+			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
+					 txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx, false);
 	} else {
 		/*
 		 * The commit has failied. For implicit commit requests restore
-		 * back the contents of the candidate DS.
+		 * back the contents of the candidate DS. For non-implicit
+		 * commit we want to allow the user to re-commit on the changes
+		 * (whether further modified or not).
 		 */
 		if (txn->commit_cfg_req->req.commit_cfg.implicit)
-			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg
-						 .dst_ds_ctx,
-					 txn->commit_cfg_req->req.commit_cfg
-						 .src_ds_ctx,
-					 false);
+			mgmt_ds_copy_dss(txn->commit_cfg_req->req.commit_cfg.src_ds_ctx,
+					 txn->commit_cfg_req->req.commit_cfg.dst_ds_ctx, false);
 	}
 
 	if (txn->commit_cfg_req->req.commit_cfg.rollback) {