[prev in list] [next in list] [prev in thread] [next in thread]
List: fedora-directory-devel
Subject: [389-devel] please review coverity fixes for ticket 403 - cleanallruv
From: Mark Reynolds <mareynol () redhat ! com>
Date: 2012-08-02 19:13:29
Message-ID: 501AD159.6060504 () redhat ! com
[Download RAW message or body]
--
Mark Reynolds
Senior Software Engineer
Red Hat, Inc
mreynolds@redhat.com
["0001-Ticket-403-cleanallruv-coverity-fixes.patch" (text/x-patch)]
From 793ff861284f3d91c84e1d2b0d25a4d0d7e07177 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@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
[Attachment #4 (text/plain)]
--
389-devel mailing list
389-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic