From 33828482372ad5b77370b17c04c6bc804f926ed4 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Mon, 24 Nov 2014 12:22:34 -0500 Subject: [PATCH 282/305] 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) Conflicts: ldap/servers/plugins/sync/sync_persist.c (cherry picked from commit 0f210a7db73836899c9eefb5e45a36e7a98c696a) Conflicts: ldap/servers/plugins/sync/sync_persist.c ldap/servers/slapd/back-ldbm/dblayer.c (cherry picked from commit 53df6b2eb8a969b379f027d4aa155bace94a1868) (cherry picked from commit c724541f1b443aa3e104b4d3314cd79c678514f8) --- 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/slapd/pw.c | 47 +++++++----- 8 files changed, 101 insertions(+), 98 deletions(-) diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c index ad929c5..6edc367 100644 --- a/ldap/servers/plugins/acl/acl.c +++ b/ldap/servers/plugins/acl/acl.c @@ -2040,9 +2040,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 7c50308..014890c 100644 --- a/ldap/servers/plugins/acl/acl_ext.c +++ b/ldap/servers/plugins/acl/acl_ext.c @@ -300,12 +300,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; } @@ -313,29 +315,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 81e486f..3fb5d79 100644 --- a/ldap/servers/plugins/acl/acllas.c +++ b/ldap/servers/plugins/acl/acllas.c @@ -1693,10 +1693,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 5d3e143..ae26380 100644 --- a/ldap/servers/plugins/replication/repl5_inc_protocol.c +++ b/ldap/servers/plugins/replication/repl5_inc_protocol.c @@ -924,7 +924,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 1cec805..3cc5cbb 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/ldap/servers/plugins/replication/repl5_replica.c @@ -1931,7 +1931,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 d9a3b26..2dcf7f0 100644 --- a/ldap/servers/plugins/replication/repl5_ruv.c +++ b/ldap/servers/plugins/replication/repl5_ruv.c @@ -693,7 +693,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 321d1a2..fa053d3 100644 --- a/ldap/servers/plugins/replication/windows_inc_protocol.c +++ b/ldap/servers/plugins/replication/windows_inc_protocol.c @@ -679,63 +679,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/slapd/pw.c b/ldap/servers/slapd/pw.c index e0d0080..0349c5a 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -2034,28 +2034,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 ) @@ -2064,11 +2069,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