Bug 610281 - fix coverity Defect Type: Control flow issues
fix coverity Defect Type: Control flow issues
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Directory Server (Show other bugs)
1.2.6
All Linux
low Severity medium
: ---
: ---
Assigned To: Noriko Hosoi
Chandrasekar Kannan
:
Depends On:
Blocks: 389_1.2.7 639035
  Show dependency treegraph
 
Reported: 2010-07-01 18:52 EDT by Noriko Hosoi
Modified: 2015-01-04 18:43 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-05-17 10:08:39 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
git patch file (9.0) (1.53 KB, patch)
2010-07-02 12:42 EDT, Noriko Hosoi
no flags Details | Diff
git patch file (9.0) (1.12 KB, patch)
2010-07-02 13:06 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.54 KB, patch)
2010-07-02 13:32 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.88 KB, patch)
2010-07-02 13:48 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.54 KB, patch)
2010-07-02 14:05 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.64 KB, patch)
2010-07-02 14:37 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.38 KB, patch)
2010-07-02 14:47 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (2.27 KB, patch)
2010-07-02 16:38 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.79 KB, patch)
2010-07-02 16:55 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (6.19 KB, patch)
2010-07-02 18:16 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (2.83 KB, patch)
2010-07-02 18:47 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (2.38 KB, patch)
2010-07-02 19:05 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.47 KB, patch)
2010-07-02 19:17 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.22 KB, patch)
2010-07-02 19:29 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.32 KB, patch)
2010-07-02 19:37 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.86 KB, patch)
2010-07-02 20:17 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.30 KB, patch)
2010-07-02 20:25 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.18 KB, patch)
2010-07-02 20:40 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.18 KB, patch)
2010-07-02 20:47 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.07 KB, patch)
2010-07-02 21:00 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.43 KB, patch)
2010-07-07 20:41 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.40 KB, patch)
2010-07-07 20:54 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.63 KB, patch)
2010-07-07 21:08 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.63 KB, patch)
2010-07-07 21:20 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git (2.78 KB, patch)
2010-07-08 13:22 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (2.43 KB, patch)
2010-07-08 13:54 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (master) (1.74 KB, patch)
2010-07-08 14:06 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (2.72 KB, patch)
2010-07-08 14:30 EDT, Noriko Hosoi
rmeggins: review-
Details | Diff
git patch file (9.0) (2.91 KB, patch)
2010-07-08 15:13 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.11 KB, patch)
2010-07-08 15:22 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (9.83 KB, patch)
2010-07-08 16:27 EDT, Noriko Hosoi
rmeggins: review+
Details | Diff
git patch file (9.0) (1.24 KB, patch)
2010-07-23 16:49 EDT, Noriko Hosoi
no flags Details | Diff
0001-Bug-610281-fix-coverity-Defect-Type-Control-flow.patch (2.31 KB, patch)
2010-08-13 16:44 EDT, Rich Megginson
nhosoi: review+
Details | Diff

  None (edit)
Description Noriko Hosoi 2010-07-01 18:52:39 EDT
Description of problem:
http://10.16.47.145:8080/defects/index.htm?projectId=10030
Click Control flow issues
Comment 1 Noriko Hosoi 2010-07-02 12:42:56 EDT
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 {
Comment 2 Noriko Hosoi 2010-07-02 13:06:10 EDT
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.
Comment 3 Noriko Hosoi 2010-07-02 13:32:28 EDT
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.
Comment 4 Noriko Hosoi 2010-07-02 13:48:25 EDT
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));
Comment 5 Noriko Hosoi 2010-07-02 14:05:08 EDT
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 );
Comment 6 Noriko Hosoi 2010-07-02 14:37:17 EDT
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;
Comment 7 Noriko Hosoi 2010-07-02 14:47:26 EDT
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.
Comment 8 Noriko Hosoi 2010-07-02 16:38:27 EDT
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;
Comment 9 Noriko Hosoi 2010-07-02 16:55:05 EDT
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 }
Comment 10 Noriko Hosoi 2010-07-02 18:16:19 EDT
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.
Comment 11 Noriko Hosoi 2010-07-02 18:47:05 EDT
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.
Comment 12 Noriko Hosoi 2010-07-02 19:05:56 EDT
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.
Comment 13 Noriko Hosoi 2010-07-02 19:17:13 EDT
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 }
Comment 14 Noriko Hosoi 2010-07-02 19:29:30 EDT
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)
Comment 15 Noriko Hosoi 2010-07-02 19:37:08 EDT
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.
Comment 16 Noriko Hosoi 2010-07-02 20:17:39 EDT
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.
Comment 17 Noriko Hosoi 2010-07-02 20:25:12 EDT
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 }
Comment 18 Noriko Hosoi 2010-07-02 20:40:27 EDT
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;
Comment 19 Noriko Hosoi 2010-07-02 20:47:09 EDT
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;
Comment 20 Noriko Hosoi 2010-07-02 21:00:50 EDT
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;
Comment 21 Noriko Hosoi 2010-07-07 20:41:54 EDT
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) {
Comment 22 Noriko Hosoi 2010-07-07 20:54:17 EDT
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.
Comment 23 Noriko Hosoi 2010-07-07 21:08:32 EDT
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.
Comment 24 Noriko Hosoi 2010-07-07 21:20:31 EDT
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,
Comment 25 Noriko Hosoi 2010-07-08 13:22:04 EDT
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.
Comment 26 Noriko Hosoi 2010-07-08 13:54:02 EDT
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.
Comment 27 Noriko Hosoi 2010-07-08 14:06:28 EDT
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.
Comment 28 Noriko Hosoi 2010-07-08 14:30:59 EDT
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)".
Comment 29 Noriko Hosoi 2010-07-08 15:13:45 EDT
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.
Comment 30 Noriko Hosoi 2010-07-08 15:22:44 EDT
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.
Comment 31 Noriko Hosoi 2010-07-08 16:27:31 EDT
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 32 Rich Megginson 2010-07-14 16:41:14 EDT
Comment on attachment 429110 [details]
git patch file (9.0)

Maybe it should be
-			if ( context_type & ACLPB_HAS_ACLCB_EVALCONTEXT ) {
?
Comment 33 Rich Megginson 2010-07-14 16:44:12 EDT
Comment on attachment 429127 [details]
git patch file (9.0)

The second comment should be /* mech cannot be NULL here */ but otherwise ok.
Comment 34 Rich Megginson 2010-07-14 16:51:16 EDT
Comment on attachment 429165 [details]
git patch file (9.0)

I guess you will now get
unused variable: txn
?
Comment 35 Noriko Hosoi 2010-07-14 17:01:51 EDT
(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                }
Comment 36 Noriko Hosoi 2010-07-23 16:49:35 EDT
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.
Comment 37 Noriko Hosoi 2010-07-23 16:50:26 EDT
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
Comment 38 Rich Megginson 2010-08-13 16:44:41 EDT
Created attachment 438753 [details]
0001-Bug-610281-fix-coverity-Defect-Type-Control-flow.patch
Comment 39 Rich Megginson 2010-08-16 17:13:29 EDT
To ssh://git.fedorahosted.org/git/389/ds.git
   b44e8f4..9174cc6  master -> master
commit 9174cc61589ef4fc554ca82077bbddabc06e617b
Author: Rich Megginson <rmeggins@redhat.com>
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

Note You need to log in before you can comment on or make changes to this bug.