From: Tim Beale <timbeale@catalyst.net.nz>
Date: Wed, 27 Sep 2017 23:10:11 +0000 (+1300)
Subject: replmd: Small refactor to replmd_check_singleval_la_conflict()
X-Git-Tag: tevent-0.9.34~146
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=831953656590c1d4725d2d798d3d6cdee8499513;p=thirdparty%2Fsamba.git

replmd: Small refactor to replmd_check_singleval_la_conflict()

Now that the code is all in one place we can refactor it to make it
slightly more readable.

- added more code comments
- tweaked the 'no conflict' return logic to try to make what it's checking
  for more obvious
- removed conflict_pdn (we can just use active_pdn instead)
- added a placeholder variable and tweaked a parameter name

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index f18415a137e..d21ae97a0eb 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7290,7 +7290,7 @@ static int replmd_delete_link_value(struct ldb_module *module,
  * Note that this is a corner-case that is unlikely to happen (but if it does
  * happen, we don't want it to break replication completely).
  *
- * @param pdn the parsed DN corresponding to the received link
+ * @param pdn_being_modified the parsed DN corresponding to the received link
  * target (note this is NULL if the link does not already exist in our DB)
  * @param pdn_list all the source object's Parsed-DNs for this attribute, i.e.
  * any existing active or inactive values for the attribute in our DB.
@@ -7304,7 +7304,7 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
 					      struct ldb_dn *src_obj_dn,
 					      struct drsuapi_DsReplicaLinkedAttribute *la,
 					      struct dsdb_dn *dsdb_dn,
-					      struct parsed_dn *pdn,
+					      struct parsed_dn *pdn_being_modified,
 					      struct parsed_dn *pdn_list,
 					      struct ldb_message_element *old_el,
 					      const struct dsdb_schema *schema,
@@ -7313,7 +7313,7 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
 					      bool *add_as_inactive)
 {
 	struct parsed_dn *active_pdn = NULL;
-	struct parsed_dn *conflict_pdn = NULL;
+	bool update_is_newer = false;
 	int ret;
 
 	/*
@@ -7328,35 +7328,41 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
 		return ret;
 	}
 
-	if (active_pdn != pdn) {
-		conflict_pdn = active_pdn;
-	}
-
-	if (conflict_pdn == NULL) {
+	/*
+	 * If no active value exists (or the received info is for the currently
+	 * active value), then no conflict exists
+	 */
+	if (active_pdn == NULL || active_pdn == pdn_being_modified) {
 		return LDB_SUCCESS;
 	}
 
-	/* resolve any single-valued link conflicts */
 	DBG_WARNING("Link conflict for %s attribute on %s\n",
 		    attr->lDAPDisplayName, ldb_dn_get_linearized(src_obj_dn));
 
-	if (replmd_link_update_is_newer(conflict_pdn, la)) {
+	/* Work out how to resolve the conflict based on which info is better */
+	update_is_newer = replmd_link_update_is_newer(active_pdn, la);
+
+	if (update_is_newer) {
 		DBG_WARNING("Using received value %s, over existing target %s\n",
 			    ldb_dn_get_linearized(dsdb_dn->dn),
-			    ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
+			    ldb_dn_get_linearized(active_pdn->dsdb_dn->dn));
 
+		/*
+		 * Delete our existing active link. The received info will then
+		 * be added (through normal link processing) as the active value
+		 */
 		ret = replmd_delete_link_value(module, replmd_private, old_el,
 					       src_obj_dn, schema, attr,
-					       seq_num, true, &conflict_pdn->guid,
-					       conflict_pdn->dsdb_dn,
-					       conflict_pdn->v);
+					       seq_num, true, &active_pdn->guid,
+					       active_pdn->dsdb_dn,
+					       active_pdn->v);
 
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
 	} else {
 		DBG_WARNING("Using existing target %s, over received value %s\n",
-			    ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
+			    ldb_dn_get_linearized(active_pdn->dsdb_dn->dn),
 			    ldb_dn_get_linearized(dsdb_dn->dn));
 
 		/*