From 33828482372ad5b77370b17c04c6bc804f926ed4 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

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