From 86cb4a8568160e5f5f1051506b8f24bb1312ff60 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 24 Nov 2014 12:22:34 -0500
Subject: [PATCH 54/55] Ticket 47965 - Fix coverity issues (2014/11/24)
12864 - nesting level adjustment - sync_presist.c
12867 - Uninitialized pointer read - repl5_replica.c
12870 - Unused value - repl5_ruv.c
12880, 12881, 12893 - Unused value - dblayer.c
12887 - nesting level adjustment - acl.c
12894 - nesting level adjustment - acl_ext.c
12898 - Logically dead code - acllas.c
12891 - Unused value - windows_inc_protocol.c
12902 - Unused value - repl5_inc_protocol.c
https://fedorahosted.org/389/ticket/47965
Reviewed by: rmeggins(Thanks!)
(cherry picked from commit da90d57c30bf93265f06499b8ea2102378a0ed12)
(cherry picked from commit 61d1211f1721355d571d570a67f36ef6baeb4866)
---
ldap/servers/plugins/acl/acl.c | 3 +-
ldap/servers/plugins/acl/acl_ext.c | 52 +++++++------
ldap/servers/plugins/acl/acllas.c | 4 -
.../plugins/replication/repl5_inc_protocol.c | 1 -
ldap/servers/plugins/replication/repl5_replica.c | 2 +-
ldap/servers/plugins/replication/repl5_ruv.c | 1 -
.../plugins/replication/windows_inc_protocol.c | 89 +++++++++++-----------
ldap/servers/plugins/sync/sync_persist.c | 8 +-
ldap/servers/slapd/back-ldbm/dblayer.c | 21 +++--
ldap/servers/slapd/pw.c | 47 +++++++-----
10 files changed, 119 insertions(+), 109 deletions(-)
diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c
index 403c5b3..f04e258 100644
--- a/ldap/servers/plugins/acl/acl.c
+++ b/ldap/servers/plugins/acl/acl.c
@@ -2106,9 +2106,10 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
** acl in the entry cache list.
*/
if (!((res_right & (SLAPI_ACL_SEARCH | SLAPI_ACL_READ)) &&
- (aci_right & (SLAPI_ACL_SEARCH | SLAPI_ACL_READ))))
+ (aci_right & (SLAPI_ACL_SEARCH | SLAPI_ACL_READ)))){
matches = ACL_FALSE;
goto acl__resource_match_aci_EXIT;
+ }
}
diff --git a/ldap/servers/plugins/acl/acl_ext.c b/ldap/servers/plugins/acl/acl_ext.c
index 126a234..0863de0 100644
--- a/ldap/servers/plugins/acl/acl_ext.c
+++ b/ldap/servers/plugins/acl/acl_ext.c
@@ -334,12 +334,14 @@ acl_operation_ext_destructor ( void *ext, void *object, void *parent )
int attr_only = 0;
PRLock *shared_lock = aclcb->aclcb_lock;
- if (aclcb->aclcb_lock ) PR_Lock ( shared_lock );
+ if (aclcb->aclcb_lock )
+ PR_Lock ( shared_lock );
else {
goto clean_aclpb;
}
if ( !aclcb->aclcb_lock ) {
- slapi_log_error (SLAPI_LOG_FATAL, plugin_name, "aclcb lock released! aclcb cache can't be refreshed\n");
+ slapi_log_error (SLAPI_LOG_FATAL, plugin_name,
+ "aclcb lock released! aclcb cache can't be refreshed\n");
PR_Unlock ( shared_lock );
goto clean_aclpb;
}
@@ -347,29 +349,29 @@ acl_operation_ext_destructor ( void *ext, void *object, void *parent )
/* We need to refresh the aclcb cache */
if ( aclpb->aclpb_state & ACLPB_UPD_ACLCB_CACHE )
acl_clean_aclEval_context ( &aclcb->aclcb_eval_context, 0 /* clean*/ );
- if ( aclpb->aclpb_prev_entryEval_context.acle_numof_attrs ) {
- c_evalContext = &aclpb->aclpb_prev_entryEval_context;
- } else {
- c_evalContext = &aclpb->aclpb_curr_entryEval_context;
- }
-
- if (( aclpb->aclpb_state & ACLPB_INCR_ACLCB_CACHE ) &&
- ! ( aclpb->aclpb_state & ACLPB_UPD_ACLCB_CACHE ))
- attr_only = 1;
-
- acl_copyEval_context ( NULL, c_evalContext, &aclcb->aclcb_eval_context, attr_only );
-
- aclcb->aclcb_aclsignature = aclpb->aclpb_signature;
- if ( aclcb->aclcb_sdn &&
- (0 != slapi_sdn_compare ( aclcb->aclcb_sdn,
- aclpb->aclpb_authorization_sdn ) ) ) {
- slapi_sdn_set_ndn_byval( aclcb->aclcb_sdn,
- slapi_sdn_get_ndn ( aclpb->aclpb_authorization_sdn ) );
- }
- aclcb->aclcb_state = 0;
- aclcb->aclcb_state |= ACLCB_HAS_CACHED_EVALCONTEXT;
-
- PR_Unlock ( shared_lock );
+ if ( aclpb->aclpb_prev_entryEval_context.acle_numof_attrs ) {
+ c_evalContext = &aclpb->aclpb_prev_entryEval_context;
+ } else {
+ c_evalContext = &aclpb->aclpb_curr_entryEval_context;
+ }
+
+ if (( aclpb->aclpb_state & ACLPB_INCR_ACLCB_CACHE ) &&
+ ! ( aclpb->aclpb_state & ACLPB_UPD_ACLCB_CACHE ))
+ attr_only = 1;
+
+ acl_copyEval_context ( NULL, c_evalContext, &aclcb->aclcb_eval_context, attr_only );
+
+ aclcb->aclcb_aclsignature = aclpb->aclpb_signature;
+ if ( aclcb->aclcb_sdn &&
+ (0 != slapi_sdn_compare( aclcb->aclcb_sdn, aclpb->aclpb_authorization_sdn )))
+ {
+ slapi_sdn_set_ndn_byval( aclcb->aclcb_sdn,
+ slapi_sdn_get_ndn ( aclpb->aclpb_authorization_sdn ) );
+ }
+ aclcb->aclcb_state = 0;
+ aclcb->aclcb_state |= ACLCB_HAS_CACHED_EVALCONTEXT;
+
+ PR_Unlock ( shared_lock );
}
clean_aclpb:
diff --git a/ldap/servers/plugins/acl/acllas.c b/ldap/servers/plugins/acl/acllas.c
index 2d0dae9..439c8f1 100644
--- a/ldap/servers/plugins/acl/acllas.c
+++ b/ldap/servers/plugins/acl/acllas.c
@@ -1726,10 +1726,6 @@ DS_LASAuthMethodEval(NSErr_t *errp, char *attr_name, CmpOp_t comparator,
} else {
rc = (matched == ACL_TRUE ? LAS_EVAL_FALSE : LAS_EVAL_TRUE);
}
- } else {
- rc = LAS_EVAL_FAIL;
- slapi_log_error( SLAPI_LOG_ACL, plugin_name,
- "Returning UNDEFINED for authmethod evaluation.\n");
}
return rc;
diff --git a/ldap/servers/plugins/replication/repl5_inc_protocol.c b/ldap/servers/plugins/replication/repl5_inc_protocol.c
index b867fc4..5cf170c 100644
--- a/ldap/servers/plugins/replication/repl5_inc_protocol.c
+++ b/ldap/servers/plugins/replication/repl5_inc_protocol.c
@@ -940,7 +940,6 @@ repl5_inc_run(Private_Repl_Protocol *prp)
/* Destroy the backoff timer, since we won't need it anymore */
backoff_delete(&prp_priv->backoff);
}
- use_busy_backoff_timer = PR_FALSE;
} else if (event_occurred(prp, EVENT_TRIGGERING_CRITERIA_MET)){
/* changes are available */
if ( prp_priv->backoff == NULL || backoff_expired (prp_priv->backoff, 60)){
diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
index dab1c4e..77663f6 100644
--- a/ldap/servers/plugins/replication/repl5_replica.c
+++ b/ldap/servers/plugins/replication/repl5_replica.c
@@ -2140,7 +2140,7 @@ done:
char *ridstr = NULL;
char *token = NULL;
char *repl_root;
- char *iter;
+ char *iter = NULL;
int i;
for(i = 0; clean_vals[i]; i++){
diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c
index 500fd7a..132c641 100644
--- a/ldap/servers/plugins/replication/repl5_ruv.c
+++ b/ldap/servers/plugins/replication/repl5_ruv.c
@@ -705,7 +705,6 @@ set_max_csn_nolock_ext(RUV *ruv, const CSN *max_csn, const char *replica_purl, P
csn_as_string(replica->csn, PR_FALSE, csn2));
return_value = RUV_COVERS_CSN;
}
- return_value = RUV_SUCCESS;
}
return return_value;
}
diff --git a/ldap/servers/plugins/replication/windows_inc_protocol.c b/ldap/servers/plugins/replication/windows_inc_protocol.c
index d62deec..ec6ca55 100644
--- a/ldap/servers/plugins/replication/windows_inc_protocol.c
+++ b/ldap/servers/plugins/replication/windows_inc_protocol.c
@@ -675,63 +675,64 @@ windows_inc_run(Private_Repl_Protocol *prp)
backoff_delete(&prp_priv->backoff);
}
else if (event_occurred(prp, EVENT_BACKOFF_EXPIRED))
- {
+ {
rc = windows_acquire_replica(prp, &ruv, 1 /* check RUV for incremental */);
use_busy_backoff_timer = PR_FALSE;
if (rc == ACQUIRE_SUCCESS)
- {
- next_state = STATE_SENDING_UPDATES;
- }
+ {
+ next_state = STATE_SENDING_UPDATES;
+ }
else if (rc == ACQUIRE_REPLICA_BUSY)
- {
- next_state = STATE_BACKOFF;
- use_busy_backoff_timer = PR_TRUE;
- }
+ {
+ next_state = STATE_BACKOFF;
+ use_busy_backoff_timer = PR_TRUE;
+ }
else if (rc == ACQUIRE_CONSUMER_WAS_UPTODATE)
- {
+ {
next_state = STATE_WAIT_CHANGES;
- }
+ }
else if (rc == ACQUIRE_TRANSIENT_ERROR)
- {
- next_state = STATE_BACKOFF;
- }
+ {
+ next_state = STATE_BACKOFF;
+ }
else if (rc == ACQUIRE_FATAL_ERROR)
- {
- next_state = STATE_STOP_FATAL_ERROR;
- }
+ {
+ next_state = STATE_STOP_FATAL_ERROR;
+ }
+
if (rc != ACQUIRE_SUCCESS)
- {
- int optype, ldaprc;
- windows_conn_get_error(prp->conn, &optype, &ldaprc);
- agmt_set_last_update_status(prp->agmt, ldaprc,
+ {
+ int optype, ldaprc;
+ windows_conn_get_error(prp->conn, &optype, &ldaprc);
+ agmt_set_last_update_status(prp->agmt, ldaprc,
prp->last_acquire_response_code, NULL);
- }
- /*
- * We either need to step the backoff timer, or
- * destroy it if we don't need it anymore.
- */
+ }
+
+ /*
+ * We either need to step the backoff timer, or
+ * destroy it if we don't need it anymore.
+ */
if (STATE_BACKOFF == next_state)
- {
- time_t next_fire_time;
- time_t now;
- /* Step the backoff timer */
- time(&now);
- next_fire_time = backoff_step(prp_priv->backoff);
- /* And go back to sleep */
- slapi_log_error(SLAPI_LOG_REPL, windows_repl_plugin_name,
- "%s: Replication session backing off for %ld seconds\n",
- agmt_get_long_name(prp->agmt),
- next_fire_time - now);
+ {
+ time_t next_fire_time;
+ time_t now;
+ /* Step the backoff timer */
+ time(&now);
+ next_fire_time = backoff_step(prp_priv->backoff);
+ /* And go back to sleep */
+ slapi_log_error(SLAPI_LOG_REPL, windows_repl_plugin_name,
+ "%s: Replication session backing off for %ld seconds\n",
+ agmt_get_long_name(prp->agmt),
+ next_fire_time - now);
- protocol_sleep(prp, PR_INTERVAL_NO_TIMEOUT);
- }
+ protocol_sleep(prp, PR_INTERVAL_NO_TIMEOUT);
+ }
else
- {
- /* Destroy the backoff timer, since we won't need it anymore */
- backoff_delete(&prp_priv->backoff);
- }
- use_busy_backoff_timer = PR_FALSE;
- }
+ {
+ /* Destroy the backoff timer, since we won't need it anymore */
+ backoff_delete(&prp_priv->backoff);
+ }
+ }
else if (event_occurred(prp, EVENT_TRIGGERING_CRITERIA_MET))
{
/* changes are available */
diff --git a/ldap/servers/plugins/sync/sync_persist.c b/ldap/servers/plugins/sync/sync_persist.c
index 8d46102..f41a10c 100644
--- a/ldap/servers/plugins/sync/sync_persist.c
+++ b/ldap/servers/plugins/sync/sync_persist.c
@@ -661,10 +661,10 @@ sync_send_results( void *arg )
ectrls = (LDAPControl **)slapi_ch_calloc(2, sizeof (LDAPControl *));
if (req->req_cookie)
sync_cookie_update(req->req_cookie, ec);
- sync_create_state_control(ec, &ectrls[0], chg_type, req->req_cookie);
- rc = slapi_send_ldap_search_entry( req->req_pblock,
- ec, ectrls,
- noattrs?noattrs:attrs, attrsonly );
+ sync_create_state_control(ec, &ectrls[0], chg_type, req->req_cookie);
+ rc = slapi_send_ldap_search_entry( req->req_pblock,
+ ec, ectrls,
+ noattrs?noattrs:attrs, attrsonly );
if (rc) {
slapi_log_error(SLAPI_LOG_CONNS, SYNC_PLUGIN_SUBSYSTEM,
"Error %d sending entry %s\n",
diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c
index f3159a9..2524355 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -4805,19 +4805,18 @@ static int checkpoint_threadmain(void *param)
for (inst_obj = objset_first_obj(li->li_instance_set);
inst_obj;
- inst_obj = objset_next_obj(li->li_instance_set, inst_obj)) {
+ inst_obj = objset_next_obj(li->li_instance_set, inst_obj))
+ {
inst = (ldbm_instance *)object_get_data(inst_obj);
rc = dblayer_get_id2entry(inst->inst_be, &db);
- if (!db) {
+ if (!db || rc ) {
continue;
}
LDAPDebug1Arg(LDAP_DEBUG_BACKLDBM, "compactdb: Compacting DB start: %s\n",
inst->inst_name);
rc = dblayer_txn_begin(inst->inst_be, NULL, &txn);
if (rc) {
- LDAPDebug1Arg(LDAP_DEBUG_ANY,
- "compactdb: transaction begin failed: %d\n",
- rc);
+ LDAPDebug1Arg(LDAP_DEBUG_ANY, "compactdb: transaction begin failed: %d\n", rc);
break;
}
rc = db->compact(db, txn.back_txn_txn, NULL/*start*/, NULL/*stop*/,
@@ -4826,12 +4825,20 @@ static int checkpoint_threadmain(void *param)
LDAPDebug(LDAP_DEBUG_ANY,
"compactdb: failed to compact %s; db error - %d %s\n",
inst->inst_name, rc, db_strerror(rc));
- rc = dblayer_txn_abort(inst->inst_be, &txn);
+ if((rc = dblayer_txn_abort(inst->inst_be, &txn))){
+ LDAPDebug(LDAP_DEBUG_ANY, "compactdb: failed to abort txn (%s) db error - %d %s\n",
+ inst->inst_name, rc, db_strerror(rc));
+ break;
+ }
} else {
LDAPDebug2Args(LDAP_DEBUG_BACKLDBM,
"compactdb: compact %s - %d pages freed\n",
inst->inst_name, c_data.compact_pages_free);
- rc = dblayer_txn_commit(inst->inst_be, &txn);
+ if((rc = dblayer_txn_commit(inst->inst_be, &txn))){
+ LDAPDebug(LDAP_DEBUG_ANY, "compactdb: failed to commit txn (%s) db error - %d %s\n",
+ inst->inst_name, rc, db_strerror(rc));
+ break;
+ }
}
}
time_of_last_comapctdb_completion = current_time(); /* seconds since epoch */
diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c
index a4d2dc6..7930be9 100644
--- a/ldap/servers/slapd/pw.c
+++ b/ldap/servers/slapd/pw.c
@@ -2011,28 +2011,33 @@ slapi_pwpolicy_make_response_control (Slapi_PBlock *pb, int seconds, int logins,
}
rc = ber_printf( ber, "{" );
- if ( seconds >= 0 || logins >= 0 ) {
- if ( seconds >= 0 ) {
- rc = ber_printf( ber, "t{ti}", LDAP_TAG_PWP_WARNING,
- LDAP_TAG_PWP_SECSLEFT,
- seconds );
- }
- else {
- rc = ber_printf( ber, "t{ti}", LDAP_TAG_PWP_WARNING,
- LDAP_TAG_PWP_GRCLOGINS,
- logins );
+ if ( rc != -1){
+ if(seconds >= 0 || logins >= 0 ) {
+ if ( seconds >= 0 ) {
+ rc = ber_printf( ber, "t{ti}", LDAP_TAG_PWP_WARNING,
+ LDAP_TAG_PWP_SECSLEFT,
+ seconds );
+ }
+ else {
+ rc = ber_printf( ber, "t{ti}", LDAP_TAG_PWP_WARNING,
+ LDAP_TAG_PWP_GRCLOGINS,
+ logins );
+ }
+ }
+ if (rc != -1){
+ if ( error >= 0 ) {
+ rc = ber_printf( ber, "te", LDAP_TAG_PWP_ERROR, error );
+ }
+ if (rc != -1){
+ rc = ber_printf( ber, "}" );
+ if ( rc != -1 )
+ {
+ rc = ber_flatten( ber, &bvp );
+ }
+ }
}
}
- if ( error >= 0 ) {
- rc = ber_printf( ber, "te", LDAP_TAG_PWP_ERROR, error );
- }
- rc = ber_printf( ber, "}" );
- if ( rc != -1 )
- {
- rc = ber_flatten( ber, &bvp );
- }
-
ber_free( ber, 1 );
if ( rc != -1 )
@@ -2041,11 +2046,11 @@ slapi_pwpolicy_make_response_control (Slapi_PBlock *pb, int seconds, int logins,
new_ctrl.ldctl_oid = LDAP_X_CONTROL_PWPOLICY_RESPONSE;
new_ctrl.ldctl_value = *bvp;
new_ctrl.ldctl_iscritical = 0;
- rc= slapi_pblock_set( pb, SLAPI_ADD_RESCONTROL, &new_ctrl );
+ rc = slapi_pblock_set( pb, SLAPI_ADD_RESCONTROL, &new_ctrl );
ber_bvfree(bvp);
}
- LDAPDebug( LDAP_DEBUG_TRACE, "<= slapi_pwpolicy_make_response_control", 0, 0, 0 );
+ LDAPDebug( LDAP_DEBUG_TRACE, "<= slapi_pwpolicy_make_response_control (%d)", rc, 0, 0 );
return (rc==-1?LDAP_OPERATIONS_ERROR:LDAP_SUCCESS);
}
--
1.9.3