From d68b40c3b5638f5160ff12584bb535dac6cb6628 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Aug 03 2012 15:46:18 +0000 Subject: Ticket 403 - cleanallruv coverity fixes Fixed coverity issues Reviewed by: richm(Thanks Rich!) (cherry picked from commit 75e5d3d1dba984f0219512d908d1a624096c7acc) --- 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..75f82ab 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/ldap/servers/plugins/replication/repl5_replica.c @@ -1825,7 +1825,7 @@ replica_check_for_tasks(Replica *r, Slapi_Entry *e) 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,20 @@ 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); } if ((clean_vals = slapi_entry_attr_get_charray(e, type_replicaAbortCleanRUV)) != NULL) @@ -1915,12 +1924,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 +1970,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 +3796,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..a03740b 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/ldap/servers/plugins/replication/repl5_replica_config.c @@ -1319,21 +1319,13 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, Slapi_Task *task, c /* 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 */ @@ -1419,7 +1411,8 @@ fail: ber_bvfree(payload); } csn_free(&maxcsn); - object_release (r); + if(task) /* only the task acquires the r obj */ + object_release (r); done: @@ -1476,7 +1469,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; } @@ -1819,7 +1812,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 +1835,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 +1844,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 = slapi_create_dn_string("cn=replica,cn=%s,cn=mapping tree,cn=config", slapi_sdn_get_dn(sdn)); /* * Add task to remote replica */ @@ -1863,6 +1856,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 +1934,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 +2065,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 +2162,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 +2174,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 +2210,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 +2269,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 +2472,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 +2496,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 +2526,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 +2614,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 +2633,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..db94ba7 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; @@ -1656,6 +1656,8 @@ free_and_return: if (mtnode_ext->replica) object_release (mtnode_ext->replica); } + if(rc) + csn_free(&maxcsn); slapi_ch_free_string(&payload); /*