replmd: Small refactor to replmd_check_singleval_la_conflict()
authorTim Beale <timbeale@catalyst.net.nz>
Wed, 27 Sep 2017 23:10:11 +0000 (12:10 +1300)
committerAndrew Bartlett <abartlet@samba.org>
Fri, 20 Oct 2017 02:05:21 +0000 (04:05 +0200)
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>
source4/dsdb/samdb/ldb_modules/repl_meta_data.c

index f18415a137e9ca0c74284070dd5a4cebd211664f..d21ae97a0ebd545b90de92a66873ffdcfc53a25b 100644 (file)
@@ -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));
 
                /*