--
Mark Reynolds
Senior Software Engineer
Red Hat, Inc
mreyno...@redhat.com

>From 793ff861284f3d91c84e1d2b0d25a4d0d7e07177 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreyno...@redhat.com>
Date: Thu, 2 Aug 2012 15:11:45 -0400
Subject: [PATCH] Ticket 403 - cleanallruv coverity fixes

Fixed coverity issues

Reviewed by: ?
---
 ldap/servers/plugins/replication/repl5_agmt.c      |    9 ++-
 ldap/servers/plugins/replication/repl5_replica.c   |   36 +++++++++---
 .../plugins/replication/repl5_replica_config.c     |   59 ++++++++++----------
 ldap/servers/plugins/replication/repl_extop.c      |    7 ++-
 4 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c
index 5a16083..5817f99 100644
--- a/ldap/servers/plugins/replication/repl5_agmt.c
+++ b/ldap/servers/plugins/replication/repl5_agmt.c
@@ -434,7 +434,8 @@ agmt_new_from_entry(Slapi_Entry *e)
 		for (i = 0; i < CLEANRIDSIZ && clean_vals[i]; i++){
 			ra->cleanruv_notified[i] = atoi(clean_vals[i]);
 		}
-		ra->cleanruv_notified[i + 1] = 0;
+		if(i <= CLEANRIDSIZ)
+		    ra->cleanruv_notified[i + 1] = 0;
 		slapi_ch_array_free(clean_vals);
 	} else {
 		ra->cleanruv_notified[0] = 0;
@@ -2587,7 +2588,7 @@ agmt_set_attrs_to_strip(Repl_Agmt *ra, Slapi_Entry *e)
     tmpstr = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaStripAttrs);
     if (NULL != tmpstr){
         if(ra->attrs_to_strip){
-            slapi_ch_array_free(&ra->attrs_to_strip);
+            slapi_ch_array_free(ra->attrs_to_strip);
         }
         ra->attrs_to_strip = slapi_str2charray_ext(tmpstr, " ", 0);
         PR_Unlock(ra->lock);
@@ -2675,7 +2676,9 @@ agmt_set_cleanruv_notified_from_entry(Repl_Agmt *ra, Slapi_Entry *e){
         for (i = 0; i < CLEANRIDSIZ && attr_vals[i]; i++){
             ra->cleanruv_notified[i] = atoi(attr_vals[i]);
         }
-        ra->cleanruv_notified[i + 1] = 0;
+        if( i <= CLEANRIDSIZ )
+            ra->cleanruv_notified[i + 1] = 0;
+        slapi_ch_array_free(attr_vals);
     } else {
         ra->cleanruv_notified[0] = 0;
     }
diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
index 27cdacd..e54c239 100644
--- a/ldap/servers/plugins/replication/repl5_replica.c
+++ b/ldap/servers/plugins/replication/repl5_replica.c
@@ -1819,13 +1819,13 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e)
         CSN *maxcsn = NULL;
         char *csnpart;
         char *iter;
-        char csnstr[CSN_STRSIZE];
+        char *csnstr = NULL;
         char *ridstr;
         char *token = NULL;
         ReplicaId rid;
         int i;
 
-        for(i = 0; clean_vals[i]; i++){
+        for(i = 0; i < CLEANRIDSIZ && clean_vals[i]; i++){
             cleanruv_data *data = NULL;
 
             /*
@@ -1863,7 +1863,8 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e)
             if(payload == NULL){
                 slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "CleanAllRUV Task: Startup: Failed to "
                     "create extended op payload, aborting task");
-                return;
+                csn_free(&maxcsn);
+                goto done;
             }
             /*
              *  Setup the data struct, and fire off the thread.
@@ -1886,12 +1887,21 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e)
                         (void *)data, PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
                         PR_UNJOINABLE_THREAD, SLAPD_DEFAULT_THREAD_STACKSIZE);
                 if (thread == NULL) {
+                    /* log an error and free everything */
                     slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV: unable to create cleanAllRUV "
                         "thread for rid(%d)\n", (int)data->rid);
+                    csn_free(&maxcsn);
+                    slapi_sdn_free(&data->sdn);
+                    ber_bvfree(data->payload);
+                    slapi_ch_free((void **)&data);
                 }
             }
         }
         r->repl_cleanruv_data[i] = NULL;
+
+done:
+        slapi_ch_array_free(clean_vals);
+        slapi_ch_free_string(&csnstr);
     }
 
     if ((clean_vals = slapi_entry_attr_get_charray(e, type_replicaAbortCleanRUV)) != NULL)
@@ -1915,12 +1925,12 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e)
                 if(rid <= 0 || rid >= READ_ONLY_REPLICA_ID){
                     slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "Abort CleanAllRUV Task: invalid replica id(%d) "
                         "aborting task.\n", rid);
-                    goto done;
+                    goto done2;
                 }
             } else {
                 slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "Abort CleanAllRUV Task: unable to parse cleanallruv "
                     "data (%s), aborting task.\n",clean_vals[i]);
-                goto done;
+                goto done2;
             }
 
             repl_root = ldap_utf8strtok_r(iter, ":", &iter);
@@ -1961,14 +1971,18 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e)
                     if (thread == NULL) {
                         slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "Abort CleanAllRUV Task: unable to create abort cleanAllRUV "
                             "thread for rid(%d)\n", (int)data->rid);
+                        slapi_sdn_free(&data->sdn);
+                        ber_bvfree(data->payload);
+                        slapi_ch_free_string(&data->repl_root);
+                        slapi_ch_free((void **)&data);
                     }
                 }
             }
         }
-    }
 
-done:
-    slapi_ch_array_free(clean_vals);
+done2:
+        slapi_ch_array_free(clean_vals);
+    }
 }
 
 /* This function updates the entry to contain information generated 
@@ -3783,8 +3797,10 @@ replica_add_cleanruv_data(Replica *r, char *val)
     PR_Lock(r->repl_lock);
 
     for (i = 0; i < CLEANRIDSIZ && r->repl_cleanruv_data[i] != NULL; i++); /* goto the end of the list */
-    r->repl_cleanruv_data[i] = slapi_ch_strdup(val); /* append to list */
-    r->repl_cleanruv_data[i + 1] = NULL;
+    if( i < CLEANRIDSIZ)
+        r->repl_cleanruv_data[i] = slapi_ch_strdup(val); /* append to list */
+    if(i <= CLEANRIDSIZ)
+        r->repl_cleanruv_data[i + 1] = NULL;
 
     PR_Unlock(r->repl_lock);
 }
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
index 89651cf..51074d7 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -1312,28 +1312,20 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, Slapi_Task *task, c
     const RUV *ruv;
     struct berval *payload = NULL;
     char *ridstr = NULL;
-    char csnstr[CSN_STRSIZE];
+    char *csnstr = NULL;
     int rc = 0;
 
     if(get_cleanruv_task_count() >= CLEANRIDSIZ){
         /* we are already running the maximum number of tasks */
         cleanruv_log(pre_task, CLEANALLRUV_ID,
     	    "Exceeded maximum number of active CLEANALLRUV tasks(%d)",CLEANRIDSIZ);
-        returntext = PR_smprintf("Exceeded maximum number of active CLEANALLRUV tasks(%d), "
-            "you must wait for one to finish.", CLEANRIDSIZ);
         return LDAP_UNWILLING_TO_PERFORM;
     }
 
     /*
      *  Grab the replica
      */
-    if(r){
-        replica = (Replica*)object_get_data (r);
-    } else {
-    	cleanruv_log(pre_task, CLEANALLRUV_ID, "Replica is NULL, aborting task");
-        rc = -1;
-        goto fail;
-    }
+    replica = (Replica*)object_get_data (r);
     /*
      *  Check if this is a consumer
      */
@@ -1376,6 +1368,7 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, Slapi_Task *task, c
     ridstr = slapi_ch_smprintf("%d:%s:%s", rid, slapi_sdn_get_dn(replica_get_root(replica)), csnstr);
     payload = create_ruv_payload(ridstr);
     slapi_ch_free_string(&ridstr);
+    slapi_ch_free_string(&csnstr);
 
     if(payload == NULL){
         cleanruv_log(pre_task, CLEANALLRUV_ID, "Failed to create extended op payload, aborting task");
@@ -1419,7 +1412,8 @@ fail:
         ber_bvfree(payload);
     }
     csn_free(&maxcsn);
-    object_release (r);
+    if(task) /* only the task acquires the r obj */
+         object_release (r);
 
 done:
 
@@ -1452,7 +1446,7 @@ replica_cleanallruv_thread(void *arg)
     Repl_Agmt *agmt = NULL;
     RUV *ruv = NULL;
     cleanruv_data *data = arg;
-    char csnstr[CSN_STRSIZE];
+    char *csnstr = NULL;
     char *returntext = NULL;
     char *rid_text = NULL;
     int found_dirty_rid = 1;
@@ -1476,7 +1470,7 @@ replica_cleanallruv_thread(void *arg)
     if(data->replica == NULL && data->repl_obj){
         data->replica = (Replica*)object_get_data(data->repl_obj);
     }
-    if( data->replica && data->repl_obj == NULL){
+    if( data->repl_obj == NULL){
         data->repl_obj = object_new(data->replica, NULL);
         free_obj = 1;
     }
@@ -1654,6 +1648,7 @@ done:
     if(data->repl_obj && free_obj){
         object_release(data->repl_obj);
     }
+    slapi_ch_free_string(&csnstr);
     slapi_sdn_free(&data->sdn);
     slapi_ch_free_string(&rid_text);
     csn_free(&data->maxcsn);
@@ -1819,7 +1814,7 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, ReplicaId rid, Slapi_Task *task)
     Repl_Connection *conn;
     ConnResult crc = 0;
     LDAP *ld;
-    Slapi_DN *dn;
+    Slapi_DN *sdn;
     struct berval *vals[2];
     struct berval val;
     LDAPMod *mods[2];
@@ -1842,7 +1837,7 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, ReplicaId rid, Slapi_Task *task)
         return;
     }
     val.bv_len = PR_snprintf(data, sizeof(data), "CLEANRUV%d", rid);
-    dn = agmt_get_replarea(agmt);
+    sdn = agmt_get_replarea(agmt);
     mod.mod_op  = LDAP_MOD_ADD|LDAP_MOD_BVALUES;
     mod.mod_type = "nsds5task";
     mod.mod_bvalues = vals;
@@ -1851,7 +1846,7 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, ReplicaId rid, Slapi_Task *task)
     val.bv_val = data;
     mods[0] = &mod;
     mods[1] = NULL;
-    repl_dn = PR_smprintf("cn=replica,cn=\"%s\",cn=mapping tree,cn=config", slapi_sdn_get_dn(dn));
+    repl_dn = PR_smprintf("cn=replica,cn=\"%s\",cn=mapping tree,cn=config", slapi_sdn_get_dn(sdn));
     /*
      *  Add task to remote replica
      */
@@ -1863,6 +1858,7 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, ReplicaId rid, Slapi_Task *task)
             agmt_get_long_name(agmt), agmt_get_hostname(agmt), rc);
     }
     slapi_ch_free_string(&repl_dn);
+    slapi_sdn_free(&sdn);
     conn_delete_internal_ext(conn);
 }
 
@@ -1940,6 +1936,9 @@ add_cleaned_rid(ReplicaId rid, Replica *r, char *maxcsn)
     char *dn;
     int rc;
 
+    if(r == NULL || maxcsn == NULL){
+        return;
+    }
     /*
      *  Write the rid & maxcsn to the config entry
      */
@@ -2068,13 +2067,12 @@ delete_aborted_rid(Replica *r, ReplicaId rid, char *repl_root){
 
     slapi_modify_internal_set_pb(pb, dn, mods, NULL, NULL, repl_get_plugin_identity (PLUGIN_MULTIMASTER_REPLICATION), 0);
     slapi_modify_internal_pb (pb);
-    slapi_pblock_destroy (pb);
     slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
     if (rc != LDAP_SUCCESS){
         slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "Abort CleanAllRUV Task: failed to remove replica "
             "config (%d), rid (%d)\n", rc, rid);
     }
-
+    slapi_pblock_destroy (pb);
     slapi_ch_free_string(&dn);
     slapi_ch_free_string(&data);
 }
@@ -2166,7 +2164,7 @@ replica_cleanall_ruv_abort(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter
     Replica *replica;
     ReplicaId rid;
     cleanruv_data *data = NULL;
-    const Slapi_DN *dn;
+    Slapi_DN *sdn;
     Object *r;
     CSN *maxcsn;
     const char *base_dn;
@@ -2178,8 +2176,6 @@ replica_cleanall_ruv_abort(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter
         /* we are already running the maximum number of tasks */
         cleanruv_log(task, ABORT_CLEANALLRUV_ID,
     	    "Exceeded maximum number of active ABORT CLEANALLRUV tasks(%d)",CLEANRIDSIZ);
-        returntext = PR_smprintf("Exceeded maximum number of active ABORT CLEANALLRUV tasks(%d), "
-            "you must wait for one to finish.", CLEANRIDSIZ);
         *returncode = LDAP_OPERATIONS_ERROR;
         return SLAPI_DSE_CALLBACK_ERROR;
     }
@@ -2216,8 +2212,8 @@ replica_cleanall_ruv_abort(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter
     /*
      *  Get the replica object
      */
-    dn = slapi_sdn_new_dn_byval(base_dn);
-    if((r = replica_get_replica_from_dn(dn)) == NULL){
+    sdn = slapi_sdn_new_dn_byval(base_dn);
+    if((r = replica_get_replica_from_dn(sdn)) == NULL){
         cleanruv_log(task, ABORT_CLEANALLRUV_ID,"Failed to find replica from dn(%s)", base_dn);
         *returncode = LDAP_OPERATIONS_ERROR;
         rc = SLAPI_DSE_CALLBACK_ERROR;
@@ -2275,6 +2271,7 @@ out:
 
     csn_free(&maxcsn);
     slapi_ch_free_string(&ridstr);
+    slapi_sdn_free(&sdn);
 
     if(rc != SLAPI_DSE_CALLBACK_OK){
         cleanruv_log(task, ABORT_CLEANALLRUV_ID, "Abort Task failed (%d)", rc);
@@ -2477,7 +2474,7 @@ replica_cleanallruv_check_maxcsn(Repl_Agmt *agmt, char *rid_text, char *maxcsn,
 {
     Repl_Connection *conn = NULL;
     LDAP *ld;
-    Slapi_DN *dn = agmt_get_replarea(agmt);
+    Slapi_DN *sdn;
     struct berval **vals;
     LDAPMessage *result = NULL, *entry = NULL;
     BerElement *ber;
@@ -2501,10 +2498,11 @@ replica_cleanallruv_check_maxcsn(Repl_Agmt *agmt, char *rid_text, char *maxcsn,
             conn_delete_internal_ext(conn);
             return -1;
         }
-        rc = ldap_search_ext_s(ld, slapi_sdn_get_dn(dn), LDAP_SCOPE_SUBTREE,
+        sdn = agmt_get_replarea(agmt);
+        rc = ldap_search_ext_s(ld, slapi_sdn_get_dn(sdn), LDAP_SCOPE_SUBTREE,
             "(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))",
             attrs, 0, NULL, NULL, NULL, 0, &result);
-        slapi_sdn_free(&dn);
+        slapi_sdn_free(&sdn);
         if(rc != LDAP_SUCCESS){
         	cleanruv_log(task, CLEANALLRUV_ID,"Failed to contact "
                 "agmt (%s) error (%d), will retry later.", agmt_get_long_name(agmt), rc);
@@ -2530,7 +2528,7 @@ replica_cleanallruv_check_maxcsn(Repl_Agmt *agmt, char *rid_text, char *maxcsn,
                             for(part_count = 1; ruv_part && part_count < 5; part_count++){
                                 ruv_part = ldap_utf8strtok_r(iter, " ", &iter);
                             }
-                            if(part_count == 5){
+                            if(part_count == 5 && ruv_part){
                                 /* we have the maxcsn */
                                 if(strcmp(ruv_part, maxcsn)){
                                     /* we are not caught up yet, free, and return */
@@ -2618,7 +2616,7 @@ replica_cleanallruv_check_ruv(Repl_Agmt *ra, char *rid_text, Slapi_Task *task)
     struct berval **vals = NULL;
     LDAPMessage *result = NULL, *entry = NULL;
     LDAP *ld = NULL;
-    Slapi_DN *dn = agmt_get_replarea(ra);
+    Slapi_DN *sdn;
     char *attrs[2];
     char *attr = NULL;
     int rc = 0, i;
@@ -2637,10 +2635,11 @@ replica_cleanallruv_check_ruv(Repl_Agmt *ra, char *rid_text, Slapi_Task *task)
             goto done;
         }
 
-        rc = ldap_search_ext_s(ld, slapi_sdn_get_dn(dn), LDAP_SCOPE_SUBTREE,
+        sdn = agmt_get_replarea(ra);
+        rc = ldap_search_ext_s(ld, slapi_sdn_get_dn(sdn), LDAP_SCOPE_SUBTREE,
             "(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))",
             attrs, 0, NULL, NULL, NULL, 0, &result);
-        slapi_sdn_free(&dn);
+        slapi_sdn_free(&sdn);
         if(rc != LDAP_SUCCESS){
             cleanruv_log(task, CLEANALLRUV_ID,"Failed to contact "
                 "agmt (%s) error (%d), will retry later.", agmt_get_long_name(ra), rc);
diff --git a/ldap/servers/plugins/replication/repl_extop.c b/ldap/servers/plugins/replication/repl_extop.c
index 88bb9e3..06551e0 100644
--- a/ldap/servers/plugins/replication/repl_extop.c
+++ b/ldap/servers/plugins/replication/repl_extop.c
@@ -1499,7 +1499,7 @@ multimaster_extop_cleanruv(Slapi_PBlock *pb)
 	PRThread *thread = NULL;
 	Replica *r = NULL;
 	cleanruv_data *data = NULL;
-	CSN *maxcsn;
+	CSN *maxcsn = NULL;
 	struct berval *extop_payload;
 	struct berval *resp_bval = NULL;
 	BerElement *resp_bere = NULL;
@@ -1622,9 +1622,10 @@ multimaster_extop_cleanruv(Slapi_PBlock *pb)
 				/* We are caught up */
 				break;
 			} else {
-				char csnstr[CSN_STRSIZE];
+				char *csnstr = NULL;
 				csn_as_string(maxcsn, PR_FALSE, csnstr);
 				slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: not ruv caught up maxcsn(%s)\n", csnstr);
+				slapi_ch_free_string(&csnstr);
 			}
 			DS_Sleep(PR_SecondsToInterval(5));
 		}
@@ -1656,6 +1657,8 @@ free_and_return:
 		if (mtnode_ext->replica)
 			object_release (mtnode_ext->replica);
 	}
+	if(rc)
+	    csn_free(&maxcsn);
 	slapi_ch_free_string(&payload);
 
 	/*
-- 
1.7.1

--
389-devel mailing list
389-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel

Reply via email to