[389-commits] [389-ds-base] 02/02: Issue 50474 - Unify result codes for add and modify of repl5 config

This is an automated email from the git hooks/post-receive script.

mhonek pushed a commit to branch 389-ds-base-1.3.9
in repository 389-ds-base.

commit ed41355447a3f90553405c41499ff653ed767010
Author: Matúš Honěk <mhonek(a)redhat.com&gt;
AuthorDate: Wed Jan 16 09:49:28 2019 +0100

    Issue 50474 - Unify result codes for add and modify of repl5 config
    
    Bug Description:
    Same constraints resulting in error are reported as different LDAP
    result codes when using different operation for adjusting these.
    
    Fix Description:
    A part of the code had not conveyed the error reason down the stack,
    therefore adding this information and returning the proper code.
    
    Fixes: https://pagure.io/389-ds-base/issue/50474
    
    Author: Matus Honek <mhonek(a)redhat.com&gt;
    
    Review by: mreynolds, spichugi (thanks!)
    
    (cherry picked from commit 9bf0fc293b4c0d89f51a3b74fd77d6448c65c02d)
---
 ldap/servers/plugins/replication/repl5.h           |  2 +-
 ldap/servers/plugins/replication/repl5_replica.c   | 71 ++++++++++++----------
 .../plugins/replication/repl5_replica_config.c     |  3 +-
 3 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/ldap/servers/plugins/replication/repl5.h
b/ldap/servers/plugins/replication/repl5.h
index e08fec7..3c7f06f 100644
--- a/ldap/servers/plugins/replication/repl5.h
+++ b/ldap/servers/plugins/replication/repl5.h
@@ -662,7 +662,7 @@ Replica *replica_new(const Slapi_DN *root);
 Replica *windows_replica_new(const Slapi_DN *root);
 /* this function should be called to construct the replica object
    during addition of the replica over LDAP */
-Replica *replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation);
+int replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool is_add_operation,
Replica **r);
 void replica_destroy(void **arg);
 int replica_subentry_update(Slapi_DN *repl_root, ReplicaId rid);
 int replica_subentry_check(Slapi_DN *repl_root, ReplicaId rid);
diff --git a/ldap/servers/plugins/replication/repl5_replica.c
b/ldap/servers/plugins/replication/repl5_replica.c
index 4e7d618..cdbcde3 100644
--- a/ldap/servers/plugins/replication/repl5_replica.c
+++ b/ldap/servers/plugins/replication/repl5_replica.c
@@ -128,8 +128,9 @@ replica_new(const Slapi_DN *root)
     e = _replica_get_config_entry(root, NULL);
     if (e) {
         errorbuf[0] = '\0';
-        r = replica_new_from_entry(e, errorbuf,
-                                   PR_FALSE /* not a newly added entry */);
+        replica_new_from_entry(e, errorbuf,
+                               PR_FALSE, /* not a newly added entry */
+                               &r);
 
         if (NULL == r) {
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_new - "
@@ -143,17 +144,17 @@ replica_new(const Slapi_DN *root)
 }
 
 /* constructs the replica object from the newly added entry */
-Replica *
-replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool is_add_operation)
+int
+replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool is_add_operation, Replica
**rp)
 {
-    int rc = 0;
     Replica *r;
+    int rc = LDAP_SUCCESS;
 
     if (e == NULL) {
         if (NULL != errortext) {
             PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, "NULL entry");
         }
-        return NULL;
+        return LDAP_OTHER;
     }
 
     r = (Replica *)slapi_ch_calloc(1, sizeof(Replica));
@@ -162,7 +163,7 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
         if (NULL != errortext) {
             PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, "Out of
memory");
         }
-        rc = -1;
+        rc = LDAP_OTHER;
         goto done;
     }
 
@@ -170,7 +171,7 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
         if (NULL != errortext) {
             PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, "failed to create
replica lock");
         }
-        rc = -1;
+        rc = LDAP_OTHER;
         goto done;
     }
 
@@ -178,7 +179,7 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
         if (NULL != errortext) {
             PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, "failed to create
replica lock");
         }
-        rc = -1;
+        rc = LDAP_OTHER;
         goto done;
     }
 
@@ -191,14 +192,17 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
 
     /* read parameters from the replica config entry */
     rc = _replica_init_from_config(r, e, errortext);
-    if (rc != 0) {
+    if (rc != LDAP_SUCCESS) {
         goto done;
     }
 
     /* configure ruv */
     rc = _replica_configure_ruv(r, PR_FALSE);
     if (rc != 0) {
+        rc = LDAP_OTHER;
         goto done;
+    } else {
+        rc = LDAP_SUCCESS;
     }
 
     /* If smallest csn exists in RUV for our local replica, it's ok to begin
iteration */
@@ -217,8 +221,12 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
          * (done by the update state event scheduled below)
          */
     }
-    if (rc != 0)
+    if (rc != 0) {
+        rc = LDAP_OTHER;
         goto done;
+    } else {
+        rc = LDAP_SUCCESS;
+    }
 
     /* ONREPL - the state update can occur before the entry is added to the DIT.
        In that case the updated would fail but nothing bad would happen. The next
@@ -239,11 +247,12 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool
is_add_operation)
     replica_check_for_tasks(r, e);
 
 done:
-    if (rc != 0 && r) {
+    if (rc != LDAP_SUCCESS && r) {
         replica_destroy((void **)&r);
     }
 
-    return r;
+    *rp = r;
+    return rc;
 }
 
 
@@ -1791,9 +1800,9 @@ _replica_check_validity(const Replica *r)
 
     if (r->repl_root == NULL || r->repl_type == 0 || r->repl_rid == 0 ||
         r->repl_csngen == NULL || r->repl_name == NULL) {
-        return -1;
+        return LDAP_OTHER;
     } else {
-        return 0;
+        return LDAP_SUCCESS;
     }
 }
 
@@ -1843,7 +1852,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                     (char *)slapi_entry_get_dn((Slapi_Entry *)e));
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "_replica_init_from_config -
%s\n",
                       errormsg);
-        return -1;
+        return LDAP_OTHER;
     }
 
     r->repl_root = slapi_sdn_new_dn_passin(val);
@@ -1853,7 +1862,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if ((val = slapi_entry_attr_get_charptr(e, attr_replicaType))) {
             if (repl_config_valid_num(attr_replicaType, val, 0, REPLICA_TYPE_UPDATABLE,
&rc, errormsg, &rtype) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             r->repl_type = rtype;
             slapi_ch_free_string(&val);
@@ -1869,7 +1878,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if ((val = slapi_entry_attr_get_charptr(e, type_replicaBackoffMin))) {
             if (repl_config_valid_num(type_replicaBackoffMin, val, 1, INT_MAX, &rc,
errormsg, &backoff_min) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             slapi_ch_free_string(&val);
         } else {
@@ -1884,7 +1893,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if ((val = slapi_entry_attr_get_charptr(e, type_replicaBackoffMax))) {
             if (repl_config_valid_num(type_replicaBackoffMax, val, 1, INT_MAX, &rc,
errormsg, &backoff_max) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             slapi_ch_free_string(&val);
         } else {
@@ -1901,7 +1910,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                     backoff_min, backoff_max);
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "_replica_init_from_config -
"
                       "%s\n", errormsg);
-        return -1;
+        return LDAP_UNWILLING_TO_PERFORM;
     } else {
         slapi_counter_set_value(r->backoff_min, backoff_min);
         slapi_counter_set_value(r->backoff_max, backoff_max);
@@ -1912,7 +1921,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if ((val = slapi_entry_attr_get_charptr(e, type_replicaProtocolTimeout))) {
             if (repl_config_valid_num(type_replicaProtocolTimeout, val, 0, INT_MAX,
&rc, errormsg, &ptimeout) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             slapi_ch_free_string(&val);
             slapi_counter_set_value(r->protocol_timeout, ptimeout);
@@ -1928,7 +1937,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if ((val = slapi_entry_attr_get_charptr(e, type_replicaReleaseTimeout))) {
             if (repl_config_valid_num(type_replicaReleaseTimeout, val, 0, INT_MAX,
&rc, errortext, &release_timeout) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             slapi_counter_set_value(r->release_timeout, release_timeout);
             slapi_ch_free_string(&val);
@@ -1952,7 +1961,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                         type_replicaPrecisePurge, precise_purging);
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
"_replica_init_from_config - "
                           "%s\n", errormsg);
-            return -1;
+            return LDAP_UNWILLING_TO_PERFORM;
         }
         slapi_ch_free_string(&precise_purging);
     } else {
@@ -1965,7 +1974,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
         if((val = slapi_entry_attr_get_charptr(e, attr_flags))) {
             if (repl_config_valid_num(attr_flags, val, 0, 1, &rc, errortext,
&rflags) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             r->repl_flags = (uint32_t)rflags;
             slapi_ch_free_string(&val);
@@ -1992,7 +2001,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
             int64_t rid;
             if (repl_config_valid_num(attr_replicaId, val, 1, 65534, &rc, errormsg,
&rid) != 0) {
                 slapi_ch_free_string(&val);
-                return -1;
+                return LDAP_UNWILLING_TO_PERFORM;
             }
             r->repl_rid = (ReplicaId)rid;
             slapi_ch_free_string(&val);
@@ -2002,7 +2011,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                         attr_replicaId, (char *)slapi_entry_get_dn((Slapi_Entry *)e));
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
                           "_replica_init_from_config - %s\n", errormsg);
-            return -1;
+            return LDAP_OTHER;
         }
     }
 
@@ -2015,7 +2024,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                     (char *)slapi_entry_get_dn((Slapi_Entry *)e));
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
                       "_replica_init_from_config - %s\n", errormsg);
-        return -1;
+        return LDAP_OTHER;
     }
     r->repl_csngen = object_new((void *)gen, (FNFree)csngen_free);
 
@@ -2033,7 +2042,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
     if ((val = slapi_entry_attr_get_charptr(e, attr_replicaBindDnGroupCheckInterval))) {
         if (repl_config_valid_num(attr_replicaBindDnGroupCheckInterval, val, -1, INT_MAX,
&rc, errormsg, &interval) != 0) {
             slapi_ch_free_string(&val);
-            return -1;
+            return LDAP_UNWILLING_TO_PERFORM;
         }
         r->updatedn_group_check_interval = interval;
         slapi_ch_free_string(&val);
@@ -2053,7 +2062,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
                         (char *)slapi_entry_get_dn((Slapi_Entry *)e), rc);
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
"_replica_init_from_config - %s\n",
                           errormsg);
-            return -1;
+            return LDAP_OTHER;
         } else
             r->new_name = PR_TRUE;
     }
@@ -2074,7 +2083,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
     if ((val = slapi_entry_attr_get_charptr(e, type_replicaPurgeDelay))) {
         if (repl_config_valid_num(type_replicaPurgeDelay, val, -1, INT_MAX, &rc,
errormsg, &interval) != 0) {
             slapi_ch_free_string(&val);
-            return -1;
+            return LDAP_UNWILLING_TO_PERFORM;
         }
         r->repl_purge_delay = interval;
         slapi_ch_free_string(&val);
@@ -2085,7 +2094,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char
*errortext)
     if ((val = slapi_entry_attr_get_charptr(e, type_replicaTombstonePurgeInterval))) {
         if (repl_config_valid_num(type_replicaTombstonePurgeInterval, val, -1, INT_MAX,
&rc, errormsg, &interval) != 0) {
             slapi_ch_free_string(&val);
-            return -1;
+            return LDAP_UNWILLING_TO_PERFORM;
         }
         r->tombstone_reap_interval = interval;
         slapi_ch_free_string(&val);
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c
b/ldap/servers/plugins/replication/repl5_replica_config.c
index b4aff9e..62bfcf6 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -224,9 +224,8 @@ replica_config_add(Slapi_PBlock *pb __attribute__((unused)),
     }
 
     /* create replica object */
-    r = replica_new_from_entry(e, errortext, PR_TRUE /* is a newly added entry */);
+    *returncode = replica_new_from_entry(e, errortext, PR_TRUE /* is a newly added entry
*/, &r);
     if (r == NULL) {
-        *returncode = LDAP_OPERATIONS_ERROR;
         goto done;
     }
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.