Created attachment 429110 [details]
git patch file (9.0)

11792 DEADCODE Triaged Unassigned Bug Minor Fix Required
acl__match_handlesFromCache() ds/ldap/servers/plugins/acl/acl.c

Comment:
These lines need to be removed:
3748 if ( context_type == ACLPB_HAS_ACLCB_EVALCONTEXT ) {
Execution cannot reach this statement "aclpb->aclpb_state &= 0xfff...".
3749 aclpb->aclpb_state &= ~ACLPB_HAS_ACLCB_EVALCONTEXT;
3750 } else {

Created attachment 429117 [details]
git patch file (9.0)

11794 DEADCODE Triaged Unassigned Bug Minor Ignore
slapi_dn_syntax_check() ds/ldap/servers/slapd/plugin_syntax.c

Comment:
Checking for the possibility of dn == NULL is not needed since it is already checked at the line 303.

Created attachment 429121 [details]
git patch file (9.0)

11795 DEADCODE Triaged Unassigned Bug Minor Fix Required
DS_LASRoleDnAttrEval() ds/ldap/servers/plugins/acl/acllas.c

Comment:
Merged the 2 lines to check matched with ACL_TRUE into one.

Created attachment 429127 [details]
git patch file (9.0)

11796 DEADCODE Triaged Unassigned Bug Minor Ignore
slapi_ldap_init_ext() ds/ldap/servers/slapd/ldaputil.c

Comment:
ldapurl is guaranteed not NULL.
323 slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_init_ext",
324 "Could not parse given LDAP URL [%s] : error [%s]\n",
325 ldapurl ? ldapurl : "NULL",
326 slapi_urlparse_err2string(rc));

11797 DEADCODE Triaged Unassigned Bug Minor Ignore
slapi_ldap_bind() ds/ldap/servers/slapd/ldaputil.c

Comment:
It is guaranteed that mech has some value at the line 755.
755 slapi_log_error(SLAPI_LOG_FATAL, "slapi_ldap_bind",
756 "Error: could not perform interactive bind for id "
757 "[%s] mech [%s]: error %d (%s)\n",
758 bindid ? bindid : "(anon)",
759 mech ? mech : "SIMPLE",
760 rc, ldap_err2string(rc));

Created attachment 429131 [details]
git patch file (9.0)

11798 DEADCODE Triaged Unassigned Bug Moderate Fix Required
cb_sasl_bind_once_s() ds/ldap/servers/plugins/chainingdb/cb_bind.c

Comment:
This is not a DEADCODE problem, but a half-baked implementation 
considering this comment:
190 /* realloc matcheddn & errmsg because the mem alloc model */
191 /* may differ from malloc
The author intended this:
184 rc = ldap_parse_result( ld, result, status, &matcheddnp2, &errmsgp2,
185                         &referrals, resctrlsp, 1 );

Created attachment 429139 [details]
git patch file (9.0)

11800 DEADCODE Triaged Unassigned Bug Minor Fix Required
cos_cache_add_defn() ds/ldap/servers/plugins/cos/cos_cache.c

Comment:
If theDef points to the allocated memory at the line 1410, ret never becomes -1. Thus, theDef never be non-NULL at 1497.  Removing 
1497         if(theDef)
1498             slapi_ch_free((void**)&theDef);

11801 DEADCODE Triaged Unassigned Bug Minor Fix Required
cos_cache_follow_pointer() ds/ldap/servers/plugins/cos/cos_cache.c

Comment:
default is not needed:
3525 default:
3526 goto bail;

Created attachment 429143 [details]
git patch file (9.0)

11802 DEADCODE Triaged Unassigned Bug Minor Ignore
dna_get_next_value() ds/ldap/servers/plugins/dna/dna.c

Comment:
Merged 2 pblock destroy codes slapi_pblock_destroy(pb) to one at the end of the function dna_get_next_value.

Created attachment 429159 [details]
git patch file (9.0)

11803 DEADCODE Triaged Unassigned Bug Minor Fix Required
_cl5GetFirstEntry() ds/ldap/servers/plugins/replication/cl5_api.c

Comment:
If rc is 0 (LDAP_SUCCESS), _cl5GetFirstEntry returns at:
5493 return CL5_SUCCESS;

These lines are not needed:
5519 if (rc != 0)
5520 {

5524 goto done;
5525 }
5526
5527 /* successfully retrieved next entry but it was out of range */
Execution cannot reach this statement "if (rc == 0){ slapi_ch_fr...".
5528 if (rc == CL5_SUCCESS)
5529 {
5530 slapi_ch_free (&(key.data));
5531 slapi_ch_free (&(data.data));
5532 rc = CL5_NOTFOUND;
5533 goto done;
5534 }

11804 DEADCODE Triaged Unassigned Bug Minor Fix Required
_cl5GetNextEntry() ds/ldap/servers/plugins/replication/cl5_api.c

Comment:
These lines are not needed:
5602 if (rc != 0)
5603 {

5608 }
5609
Execution cannot reach this statement "return rc;".
5610 return rc;

Created attachment 429165 [details]
git patch file (9.0)

11805 DEADCODE Triaged Unassigned Bug Minor Ignore
clcache_load_buffer_bulk() ds/ldap/servers/plugins/replication/cl5_clcache.c

Comment:
Experimented the transaction control, but it was not adopted.
383 /* txn control seems not improving anything so turn it off */

Comment out these lines.
418 if ( txn ) {
Execution cannot reach this statement "(*txn->commit)(txn, 256U);".
419 txn->commit ( txn, DB_TXN_NOSYNC );
420 }

Created attachment 429183 [details]
git patch file (9.0)

11806 DEADCODE Triaged Unassigned Bug Moderate Fix Required
agmt_set_last_init_status() ds/ldap/servers/plugins/replication/repl5_agmt.c

Comment:
macro NSDS50_REPL_REPLICA_READY is 0
repl5.h:#define NSDS50_REPL_REPLICA_READY 0x00 /* Replica ready, go ahead */

Because of this if expression:
1991 else if (replrc != 0)
"Replica acquired successfully" never be set to last_init_status.
Removed (replrc == NSDS50_REPL_REPLICA_READY) checking from the
else if (replrc != 0) clause and added "Replica acquired successfully"
to the string to print message since replrc == NSDS50_REPL_REPLICA_READY
there.

11807 DEADCODE Triaged Unassigned Bug Moderate Fix Required
agmt_set_last_update_status() ds/ldap/servers/plugins/replication/repl5_agmt.c

Comment:
macro NSDS50_REPL_REPLICA_READY is 0
repl5.h:#define NSDS50_REPL_REPLICA_READY 0x00 /* Replica ready, go ahead */

Because of this if expression:
1991 else if (replrc != 0)
"Replica acquired successfully" never be set to last_update_status.
Removed (replrc == NSDS50_REPL_REPLICA_READY) checking from the
else if (replrc != 0) clause and added "Replica acquired successfully"
to the string to print message since replrc == NSDS50_REPL_REPLICA_READY
there.

Created attachment 429187 [details]
git patch file (9.0)

11808 DEADCODE Triaged Unassigned Bug Minor Fix Required
replication_multimaster_plugin_init() ds/ldap/servers/plugins/replication/repl5_init.c

Comment:
There used to be an initializing replica hash code between the ine 573 and 575:
572 /* initialize replica hash - has to be done before mapping tree is
573 initialized so we can't do it in the start function */

575 if (rc != 0)
576 {
Execution cannot reach this statement "slapi_log_error(0, repl_plu...".
577 slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name,
578 "replication_multimaster_plugin_init: failed to initialize replica hash\n");
579 return -1;
580 }

The initializing replica hash code was moved but the comment and the result checking code was left. Removing them.

Also, setting non 0 value to a static int variable multimaster_initialised if the plugin registration was successful.

Created attachment 429188 [details]
git patch file (9.0)

11809 DEADCODE Triaged Unassigned Bug Minor Ignore
_replica_reap_tombstones() ds/ldap/servers/plugins/replication/repl5_replica.c

Comment:
If replica_name is NULL, _replica_reap_tombstones has returned at the line 2460. Thus there is no need to check "replica_name" is NULL or not at line 2555.

11810 DEADCODE Triaged Unassigned Bug Moderate Fix Required
replica_check_for_data_reload() ds/ldap/servers/plugins/replication/repl5_replica.c

Comment:
At the line 1478, !cl_cover_be is always true. Therefore, there is no possibility that "<" is chosen in slapi_log_error.

Created attachment 429189 [details]
git patch file (9.0)

11811 DEADCODE Triaged Unassigned Bug Minor Fix Required
decode_total_update_extop() ds/ldap/servers/plugins/replication/repl5_total.c

Comment:
dn_csn is declared and initialized, but not really used. 
We are removing these lines:
710 CSN *dn_csn = NULL;

816 if (NULL != dn_csn)
817 {
Execution cannot reach this statement "csn_free(&dn_csn);".
818 csn_free(&dn_csn);
819 }

Created attachment 429192 [details]
git patch file (9.0)

11812 DEADCODE Triaged Unassigned Bug Moderate Fix Required
repl_objset_destroy() ds/ldap/servers/plugins/replication/repl_objset.c

Comment:
This is not a deadcode, rather a typo.

It looks repl_objset_destroy never be called.
$ egrep repl_objset_destroy *[ch]
repl_objset.c:repl_objset_destroy(Repl_Objset **o, time_t maxwait, FNFree panic_fn)
repl_objset.h:void repl_objset_destroy(Repl_Objset **o, time_t maxwait, FNFree panic_fn);

If it is called with panic_fn, it'll crash the server at the line 184 since co is NULL.
184 panic_fn(co->data);

The line 182 is supposed to be:
182 if ((co = llistGetFirst((*o)->objects, &cookie)) != NULL)

Created attachment 429193 [details]
git patch file (9.0)

11813 DEADCODE Triaged Unassigned Bug Minor Fix Required
send_dirsync_search() ds/ldap/servers/plugins/replication/windows_connection.c

Comment:
op_string points to a static string:
731 op_string = "search";
We don't need to check op_string in slapi_log_error.

Created attachment 429199 [details]
git patch file (9.0)

11814 DEADCODE Triaged Unassigned Bug Moderate Fix Required
string_filter_sub() ds/ldap/servers/plugins/syntaxes/string.c

Comment:
A code to update tmpbufsize was missing. This "tpbufsize = len + 1;" is needed before slapi_ch_realloc.
351 tmpbufsize = len + 1;
352 tmpbuf = (char *) slapi_ch_realloc( tmpbuf, tmpbufsize );

Also, if (len < tmpbufsize) were true (could not be true since
tmpbufsize never have been set), bvp->bv_val was copied to buf
which is not long enough for bvp->bv_val.  The bug was also
fixed.

Created attachment 429201 [details]
git patch file (9.0)

11815 DEADCODE Triaged Unassigned Bug Minor Fix Required
distinguishedname_validate() ds/ldap/servers/plugins/syntaxes/validate.c

Comment:
A variable val_copy is declared and initialized, but not used. We can get rid of these lines:
364 char *val_copy = NULL;
403 if (val_copy) {
Execution cannot reach this statement "slapi_ch_free_string(&val_c...".
404 slapi_ch_free_string(&val_copy);
405 }

Created attachment 429202 [details]
git patch file (9.0)

11816 DEADCODE Triaged Unassigned Bug Moderate Fix Required
NS7bitAttr_Init() ds/ldap/servers/plugins/uiduniq/7bit.c

Comment:
NS7bitAttr_Init declared err twice.  One at the top and another in BEGIN - END (do - while loop).  The second err in the do - while loop is trashed when it gets out of the loop.  Regardless of the result in the do - while loop, err = 0 (success) was returned to the caller.

We are removing the second err in the BEGIN (== do - while) scope.
692 int err;

Created attachment 429203 [details]
git patch file (9.0)

11817 DEADCODE Triaged Unassigned Bug Moderate Fix Required
NSUniqueAttr_Init() ds/ldap/servers/plugins/uiduniq/uid.c

Comment:
NSUniqueAttr_Init declared err twice.  One at the top and another
in BEGIN - END (do - while loop).  The second err in the do - while 
loop is trashed when it gets out of the loop.  Regardless of the 
result in the do - while loop, err = 0 (success) was returned to 
the caller.

We are removing the second err in the BEGIN (== do - while) scope.
945 int err;

Created attachment 429205 [details]
git patch file (9.0)

11818 DEADCODE Triaged Unassigned Bug Minor Fix Required
agt_mopen_stats() ds/ldap/servers/slapd/agtmmap.c

Comment:
Removing the unreachable statement:
Execution cannot reach this statement "return 0;".
313 return 0;

Created attachment 430208 [details]
git patch file (9.0)

11820 DEADCODE Triaged Unassigned Bug Minor Fix Required
idl_new_delete_key() ds/ldap/servers/slapd/back-ldbm/idl_new.c

Comment:
tmpid is no longer used. Since we don't define DB_ALLIDS_ON_WRITE, ALLID has no chance to be stored in the db. But the code should be fixed as follows:
Remove:
480 ID tmpid = 0;
The 496 must be
496 if (id == ALLID) {

Created attachment 430209 [details]
git patch file (9.0)

11821 DEADCODE Triaged Unassigned Bug Minor Fix Required
allinstance_set_busy() ds/ldap/servers/slapd/back-ldbm/misc.c

Comment:
objset_next_obj releases the previous object internally. Thus, there is no leak.

Just "if (inst_obj) object_release(inst_obj);" is not needed.

11822 DEADCODE Triaged Unassigned Bug Minor Fix Required
allinstance_set_not_busy() ds/ldap/servers/slapd/back-ldbm/misc.c

Comment:
objset_next_obj releases the previous object internally. Thus, there is no leak.

Just "if (inst_obj) object_release(inst_obj);" is not needed.

Created attachment 430211 [details]
git patch file (9.0)

11825 DEADCODE Triaged Unassigned Bug Minor Fix Required
ldbm_back_modrdn() ds/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

Comment:
It was checking if (parententry && parententry->ep_entry) in the if ( parententry == NULL ) clause.  Removed the unnecessary code.

Created attachment 430212 [details]
git patch file (9.0)

11826 DEADCODE Triaged Unassigned Bug Minor Fix Required
ldbm_back_search() ds/ldap/servers/slapd/back-ldbm/ldbm_search.c

Comment:
On this path, the condition "abandoned" cannot be true.
  504   return ldbm_back_search_cleanup(pb, li, sort_control,
  505                    (abandoned?-1:LDAP_PROTOCOL_ERROR),
  506                    "Sort Response Control", -1,
  507                    &basesdn, &vlv_request_control);
Line 505 should be
  505                    LDAP_PROTOCOL_ERROR,

Created attachment 430428 [details]
git

11829 DEADCODE Triaged Unassigned Bug Minor Fix Required
ldbm_back_start() ds/ldap/servers/slapd/back-ldbm/start.c

Comment:
The code meant the autosized cache should be reduced 25% 
if the calculated size was less than 500MB for the overhead 
of libdb.  If larger than 500MB, the overhead is relatively 
small and can be ignored.  The code should have calculated 
the size before comparing it to 500MB.

Created attachment 430435 [details]
git patch file (9.0)

11831 DEADCODE Triaged Unassigned Bug Minor Fix Required
config_set_value() ds/ldap/servers/slapd/libglobs.c

Comment:
The config_set_value meant to set various values (e.g. "off" and "unknown") depending upon the config_var_type, but this code overrides the spec and set the empty string to all cases.
-    /* for null values, just set the attr value to the empty
-       string */
-    if (!value) {
-        slapi_entry_attr_set_charptr(e, cgas->attr_name, "");
-        return;
-    }
This patch removes the above blind empty string setting and relies on the values in each config_var_type case.  Plus, adding the NULL value check to CONFIG_CHARRAY.

Created attachment 430438 [details]
git patch file (master)

11832 DEADCODE Triaged Unassigned Bug Minor Fix Required
do_modrdn() ds/ldap/servers/slapd/modrdn.c

Comment:
At the line 201 and 212, the condition "rawnewsuperior" cannot be false.
  201    rawnewsuperior?rawnewsuperior:"",
  212    rawnewsuperior?rawnewsuperior:"",
This patch is removing the checks.

Created attachment 430442 [details]
git patch file (9.0)

11834 DEADCODE Triaged Unassigned Bug Minor Fix Required
write_function() ds/ldap/servers/slapd/daemon.c

Comment:
The location of checking for sentbytes is not correct.
Execution cannot reach this statement "if (sentbytes < count){ {...".
 1724            } else if (sentbytes < count) {
 1725                LDAPDebug(LDAP_DEBUG_CONNS,
It should not be "else" of checking for "bytes".
Moving the check after "else if (sentbytes > count)".

Created attachment 430445 [details]
git patch file (9.0)

11841 DEADCODE Triaged Unassigned Bug Minor Fix Required
sasl_map_new_private() ds/ldap/servers/slapd/sasl_map.c

Comment:
This new_lock NULL checking is not needed.
  On this path, the condition "NULL == new_lock" cannot be true.
   97        if (NULL == new_lock) {
  Execution cannot reach this statement "slapi_ch_free((void**)new_p...".
   98                slapi_ch_free((void**)new_priv);
   99                return NULL;
  100        }

11842 DEADCODE Triaged Unassigned Bug Minor Fix Required
sasl_map_insert_list_entry() ds/ldap/servers/slapd/sasl_map.c

Comment:
In sasl_map_insert_list_entry, /* Check to see if it's here already */ was not implemented.  The variable ishere pointed by coverity should have been prepared for the purpose.  Implementing a helper function sasl_map_cmp_data.

Created attachment 430450 [details]
git patch file (9.0)

11843 DEADCODE Triaged Unassigned Bug Minor Fix Required
mm_init() ds/ldap/servers/slapd/tools/mmldif.c

Comment:
A variable tailorfile is not used.
Remove it.

Created attachment 430462 [details]
git patch file (9.0)

11958 NO_EFFECT Triaged Unassigned Bug Moderate Fix Required
do_bind() ds/ldap/servers/slapd/bind.c

Comment:
slapi_dn_normalize_ext() may return a negative value but rc is of type ber_tag_t which is an unsigned int.  Introducing "ber_tag_t ber_rc" just for ber functions and "int rc" for the rest.

Comment on attachment 429110 [details]
git patch file (9.0)

Maybe it should be
-			if ( context_type & ACLPB_HAS_ACLCB_EVALCONTEXT ) {
?

Comment on attachment 429127 [details]
git patch file (9.0)

The second comment should be /* mech cannot be NULL here */ but otherwise ok.

Comment on attachment 429165 [details]
git patch file (9.0)

I guess you will now get
unused variable: txn
?

(In reply to comment #32)
> (From update of attachment 429110 [details])
> Maybe it should be
> -   if ( context_type & ACLPB_HAS_ACLCB_EVALCONTEXT ) {
> ?    

After the line 3738, context_type is either ACLPB_EVALCONTEXT_ACLCB (== 2) or ACLPB_EVALCONTEXT_PREV (== 3).  So, at the line 3748, context_type cannot be ACLPB_HAS_ACLCB_EVALCONTEXT (== 32768) and the bit also cannot be set, I think.

 3732        if ( aclpb->aclpb_state & ACLPB_HAS_ACLCB_EVALCONTEXT ) {
After this line, the value of "context_type" is equal to 3.
Assigning: "context_type" = "3".
 3733                context_type = ACLPB_EVALCONTEXT_ACLCB;
 3734                c_evalContext = &aclpb->aclpb_prev_opEval_context;
 3735         } else {
After this line, the value of "context_type" is equal to 2.
Assigning: "context_type" = "2".
 3736                context_type =  ACLPB_EVALCONTEXT_PREV;
 3737                c_evalContext = &aclpb->aclpb_prev_entryEval_context;
 3738        }
 3739
 3740
 3741        if ( aclpb->aclpb_res_type & (ACLPB_NEW_ENTRY | ACLPB_EFFECTIVE_RIGHTS) ) { 
 3742                aclpb->aclpb_state |= ACLPB_MATCHES_ALL_ACLS;
 3743                ret_val =  acl__scan_match_handles ( aclpb, context_type );
 3744                if (  -1 == ret_val ) {
 3745                        aclpb->aclpb_state &= ~ACLPB_MATCHES_ALL_ACLS;
 3746                        aclpb->aclpb_state |= ACLPB_UPD_ACLCB_CACHE;
 3747                        /* Did not match */
On this path, the condition "context_type == " cannot be true.
 3748                        if ( context_type == ACLPB_HAS_ACLCB_EVALCONTEXT ) {
Execution cannot reach this statement "aclpb->aclpb_state &= 0xfff...".
 3749                                aclpb->aclpb_state &= ~ACLPB_HAS_ACLCB_EVALCONTEXT;
 3750                        } else {
 3751                                aclpb->aclpb_state |= ACLPB_COPY_EVALCONTEXT;
 3752                                c_evalContext->acle_numof_tmatched_handles = 0;
 3753                        }
 3754                }

Created attachment 434066 [details]
git patch file (9.0)

Revised patch 429110.

11792 DEADCODE Triaged Unassigned Bug Minor Fix Required
acl__match_handlesFromCache() ds/ldap/servers/plugins/acl/acl.c

Comment:
Looks like a simple copy & paste bug.  Replaced the macro to
compare ACLPB_HAS_ACLCB_EVALCONTEXT with ACLPB_EVALCONTEXT_ACLCB.

Reviewed by Rich (Thank you!!)

Pushed to master.

$ git push
Counting objects: 254, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (209/209), done.
Writing objects: 100% (209/209), 28.82 KiB, done.
Total 209 (delta 149), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   482ba92..5ae0c1b  master -> master

Created attachment 438753 [details]
0001-Bug-610281-fix-coverity-Defect-Type-Control-flow.patch

To ssh://git.fedorahosted.org/git/389/ds.git
   b44e8f4..9174cc6  master -> master
commit 9174cc61589ef4fc554ca82077bbddabc06e617b
Author: Rich Megginson <rmeggins>
Date:   Fri Aug 13 14:32:22 2010 -0600
    Bug Description: fix coverity Defect Type: Control flow issues - daemon.c:wr
ite_function()
    Reviewed by: nhosoi (Thanks!)
    Branch: master
    Fix Description: sentbytes < count is ok in the non error case - it just mea
ns
    we need to send some more data.  Move the checking to the error case so we
    can print the number of bytes sent and expected.
    Platforms tested: RHEL5 x86_64, Fedora 14 x86_64
    Flag Day: no
    Doc impact: no