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