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!)

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;			
+		}
 	}
 
 	

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:

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;

ldap/servers/plugins/replication/repl5_inc_protocol.c

@@ -922,7 +922,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)){

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++){

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;
 }

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 */

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",

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 */

ldap/servers/slapd/pw.c

@@ -2024,28 +2024,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 )
@@ -2054,11 +2059,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);
 }