[prev in list] [next in list] [prev in thread] [next in thread]
List: fedora-directory-devel
Subject: Re: [389-devel] please review Coverity Fixes
From: Mark Reynolds <mareynol () redhat ! com>
Date: 2012-06-13 22:45:44
Message-ID: 4FD91818.9030306 () redhat ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Yes they should be! Nice catch.
Committing...
ldap/servers/plugins/replication/cl5_clcache.c | 1 +
.../plugins/replication/repl5_replica_config.c | 29
+++++++++++++-----
ldap/servers/plugins/rootdn_access/rootdn_access.c | 32
++++++++++++--------
ldap/servers/slapd/back-ldbm/cache.c | 24 +++++++++++----
ldap/servers/slapd/back-ldbm/dblayer.c | 5 +--
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 6 +--
6 files changed, 63 insertions(+), 34 deletions(-)
git push origin master
Counting objects: 29, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 1.92 KiB, done.
Total 15 (delta 12), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
8f21ac8..c446a69 master -> master
On 06/13/2012 06:22 PM, Noriko Hosoi wrote:
> Hi Mark,
>
> diff --git a/ldap/servers/plugins/rootdn_access/rootdn_access.c
> b/ldap/servers/p
> lugins/rootdn_access/rootdn_access.c
> index 7fb5615..23e33dd 100644
> --- a/ldap/servers/plugins/rootdn_access/rootdn_access.c
> +++ b/ldap/servers/plugins/rootdn_access/rootdn_access.c
> [...]
>
> static int
> rootdn_load_config(Slapi_PBlock *pb)
> {
> Slapi_Entry *e = NULL;
> *char *openTime, *closeTime;* <== These variables should be initialized?
> char hour[3], min[3];
>
> The other fixes look good to me.
> --noriko
>
> Mark Reynolds wrote:
>> Attached...
>>
>> Thanks,
>> Mark
>>
>>
>>
>> --
>> 389-devel mailing list
>> 389-devel@lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
>
>
> --
> 389-devel mailing list
> 389-devel@lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
--
Mark Reynolds
Senior Software Engineer
Red Hat, Inc
mreynolds@redhat.com
[Attachment #5 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Yes they should be! Nice catch.<br>
<br>
Committing...<br>
<br>
ldap/servers/plugins/replication/cl5_clcache.c | 1 +<br>
.../plugins/replication/repl5_replica_config.c | 29
+++++++++++++-----<br>
ldap/servers/plugins/rootdn_access/rootdn_access.c | 32
++++++++++++--------<br>
ldap/servers/slapd/back-ldbm/cache.c | 24
+++++++++++----<br>
ldap/servers/slapd/back-ldbm/dblayer.c | 5 +--<br>
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 6 +--<br>
6 files changed, 63 insertions(+), 34 deletions(-)<br>
<br>
git push origin master<br>
Counting objects: 29, done.<br>
Delta compression using up to 4 threads.<br>
Compressing objects: 100% (15/15), done.<br>
Writing objects: 100% (15/15), 1.92 KiB, done.<br>
Total 15 (delta 12), reused 0 (delta 0)<br>
To ssh://git.fedorahosted.org/git/389/ds.git<br>
8f21ac8..c446a69 master -> master<br>
<br>
<br>
On 06/13/2012 06:22 PM, Noriko Hosoi wrote:
<blockquote cite="mid:4FD91290.90704@redhat.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Hi Mark,<br>
<br>
diff --git a/ldap/servers/plugins/rootdn_access/rootdn_access.c
b/ldap/servers/p<br>
lugins/rootdn_access/rootdn_access.c<br>
index 7fb5615..23e33dd 100644<br>
--- a/ldap/servers/plugins/rootdn_access/rootdn_access.c<br>
+++ b/ldap/servers/plugins/rootdn_access/rootdn_access.c<br>
[...]<br>
<br>
static int<br>
rootdn_load_config(Slapi_PBlock *pb)<br>
{<br>
Slapi_Entry *e = NULL;<br>
<font color="#990000"><b>char *openTime, *closeTime;</b></font>
<== These variables should be initialized?<br>
char hour[3], min[3];<br>
<br>
The other fixes look good to me.<br>
--noriko<br>
<br>
Mark Reynolds wrote:
<blockquote cite="mid:4FD90A37.4090401@redhat.com" type="cite">Attached...
<br>
<br>
Thanks, <br>
Mark <br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">--
389-devel mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" \
href="mailto:389-devel@lists.fedoraproject.org">389-devel@lists.fedoraproject.org</a> \
<a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="https://admin.fedoraproject.org/mailman/listinfo/389-devel">https://admin.fedoraproject.org/mailman/listinfo/389-devel</a></pre>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">--
389-devel mailing list
<a class="moz-txt-link-abbreviated" \
href="mailto:389-devel@lists.fedoraproject.org">389-devel@lists.fedoraproject.org</a> \
<a class="moz-txt-link-freetext" \
href="https://admin.fedoraproject.org/mailman/listinfo/389-devel">https://admin.fedoraproject.org/mailman/listinfo/389-devel</a></pre>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Mark Reynolds
Senior Software Engineer
Red Hat, Inc
<a class="moz-txt-link-abbreviated" \
href="mailto:mreynolds@redhat.com">mreynolds@redhat.com</a></pre> </body>
</html>
["0001-COVERITY-FIXES.patch" (text/x-patch)]
From 2eb1a276170a6f81b1b9c6934d2bb877d0f708c1 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Wed, 13 Jun 2012 18:41:47 -0400
Subject: [PATCH] COVERITY FIXES
12762
12763
12764
12765
12766
12767
12768
12769
12771
Reviewed by: Noriko & Richm (Thanks!)
---
ldap/servers/plugins/replication/cl5_clcache.c | 1 +
.../plugins/replication/repl5_replica_config.c | 29 +++++++++++++-----
ldap/servers/plugins/rootdn_access/rootdn_access.c | 32 ++++++++++++--------
ldap/servers/slapd/back-ldbm/cache.c | 24 +++++++++++----
ldap/servers/slapd/back-ldbm/dblayer.c | 5 +--
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 6 +--
6 files changed, 63 insertions(+), 34 deletions(-)
diff --git a/ldap/servers/plugins/replication/cl5_clcache.c \
b/ldap/servers/plugins/replication/cl5_clcache.c index 5816837..202cb64 100644
--- a/ldap/servers/plugins/replication/cl5_clcache.c
+++ b/ldap/servers/plugins/replication/cl5_clcache.c
@@ -680,6 +680,7 @@ clcache_skip_change ( CLC_Buffer *buf )
*/
skip = 0;
}
+ csn_free(&cons_maxcsn);
break;
}
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c \
b/ldap/servers/plugins/replication/repl5_replica_config.c index 8d338c0..e5c7035 \
100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -1176,7 +1176,7 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, \
char *returntext) {
PRThread *thread = NULL;
Repl_Connection *conn;
- Replica *replica = (Replica*)object_get_data (r);
+ Replica *replica;
Object *agmt_obj;
Repl_Agmt *agmt;
ConnResult crc;
@@ -1189,7 +1189,16 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, \
char *returntext) int rc = 0;
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: cleaning rid \
(%d)...\n",(int)rid);
- set_cleaned_rid(rid);
+
+ /*
+ * Grab the replica
+ */
+ if(r){
+ replica = (Replica*)object_get_data (r);
+ } else {
+ slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: replica is \
NULL, aborting task\n"); + return -1;
+ }
/*
* Create payload
*/
@@ -1197,9 +1206,13 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, \
char *returntext) payload = create_ruv_payload(ridstr);
if(payload == NULL){
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: failed to \
create ext op payload, aborting task\n");
- goto done;
+ slapi_ch_free_string(&ridstr);
+ return -1;
}
+
+ set_cleaned_rid(rid);
+
agmt_obj = agmtlist_get_first_agreement_for_replica (replica);
while (agmt_obj)
{
@@ -1245,11 +1258,12 @@ replica_execute_cleanall_ruv_task (Object *r, ReplicaId rid, \
char *returntext) agmt_obj = agmtlist_get_next_agreement_for_replica (replica, \
agmt_obj); }
-done:
-
- if(payload)
+ /*
+ * We're done with the payload, free it.
+ */
+ if(payload){
ber_bvfree(payload);
-
+ }
slapi_ch_free_string(&ridstr);
/*
@@ -1277,7 +1291,6 @@ done:
slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: unable to \
create cleanAllRUV " "monitoring thread. Aborting task.\n");
}
-
} else if(r == 0){ /* success */
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "cleanAllRUV_task: Successfully \
Finished.\n"); } else {
diff --git a/ldap/servers/plugins/rootdn_access/rootdn_access.c \
b/ldap/servers/plugins/rootdn_access/rootdn_access.c index 7fb5615..19e578c 100644
--- a/ldap/servers/plugins/rootdn_access/rootdn_access.c
+++ b/ldap/servers/plugins/rootdn_access/rootdn_access.c
@@ -217,7 +217,8 @@ static int
rootdn_load_config(Slapi_PBlock *pb)
{
Slapi_Entry *e = NULL;
- char *openTime, *closeTime;
+ char *openTime = NULL;
+ char *closeTime = NULL;
char hour[3], min[3];
int result = 0;
int i;
@@ -243,7 +244,8 @@ rootdn_load_config(Slapi_PBlock *pb)
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: "
"invalid rootdn-days-allowed value (%s), must be all letters, \
and comma separators\n",closeTime); slapi_ch_free_string(&daysAllowed);
- return -1;
+ result = -1;
+ goto free_and_return;
}
daysAllowed = strToLower(daysAllowed);
}
@@ -251,14 +253,14 @@ rootdn_load_config(Slapi_PBlock *pb)
if (strcspn(openTime, "0123456789")){
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: "
"invalid rootdn-open-time value (%s), must be all \
digits\n",openTime);
- slapi_ch_free_string(&openTime);
- return -1;
+ result = -1;
+ goto free_and_return;
}
if(strlen(openTime) != 4){
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: "
"invalid format for rootdn-open-time value (%s). Should be \
HHMM\n", openTime);
- slapi_ch_free_string(&openTime);
- return -1;
+ result = -1;
+ goto free_and_return;
}
/*
* convert the time to all seconds
@@ -271,14 +273,14 @@ rootdn_load_config(Slapi_PBlock *pb)
if (strcspn(closeTime, "0123456789")){
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: "
"invalid rootdn-open-time value (%s), must be all digits, and \
should be HHMM\n",closeTime);
- slapi_ch_free_string(&closeTime);
- return -1;
+ result = -1;
+ goto free_and_return;
}
if(strlen(closeTime) != 4){
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: "
"invalid format for rootdn-open-time value (%s), should be \
HHMM\n", closeTime);
- slapi_ch_free_string(&closeTime);
- return -1;
+ result = -1;
+ goto free_and_return;
}
/*
* convert the time to all seconds
@@ -294,13 +296,15 @@ rootdn_load_config(Slapi_PBlock *pb)
"there must be a open and a close time\n");
slapi_ch_free_string(&closeTime);
slapi_ch_free_string(&openTime);
- return -1;
+ result = -1;
+ goto free_and_return;
}
if(close_time && open_time && close_time <= open_time){
/* Make sure the closing time is greater than the open time */
slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, \
"rootdn_load_config: " "the close time must be greater than the open time\n");
result = -1;
+ goto free_and_return;
}
if(hosts){
for(i = 0; hosts[i] != NULL; i++){
@@ -370,13 +374,15 @@ rootdn_load_config(Slapi_PBlock *pb)
}
}
}
- slapi_ch_free_string(&openTime);
- slapi_ch_free_string(&closeTime);
} else {
/* failed to get the plugin entry */
result = -1;
}
+free_and_return:
+ slapi_ch_free_string(&openTime);
+ slapi_ch_free_string(&closeTime);
+
slapi_log_error(SLAPI_LOG_PLUGIN, ROOTDN_PLUGIN_SUBSYSTEM, "<-- \
rootdn_load_config (%d)\n", result);
return result;
diff --git a/ldap/servers/slapd/back-ldbm/cache.c \
b/ldap/servers/slapd/back-ldbm/cache.c index 045b1eb..1c81a1b 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -1058,7 +1058,9 @@ static int entrycache_replace(struct cache *cache, struct \
backentry *olde, }
if (!add_hash(cache->c_idtable, &(newe->ep_id), sizeof(ID), newe, NULL)) {
LOG("entry cache replace: can't add id\n", 0, 0, 0);
- remove_hash(cache->c_dntable, (void *)newndn, strlen(newndn));
+ if(remove_hash(cache->c_dntable, (void *)newndn, strlen(newndn)) == 0){
+ LOG("entry cache replace: failed to remove dn table\n", 0, 0, 0);
+ }
PR_Unlock(cache->c_mutex);
return 1;
}
@@ -1066,8 +1068,12 @@ static int entrycache_replace(struct cache *cache, struct \
backentry *olde,
if (newuuid && !add_hash(cache->c_uuidtable, (void *)newuuid, strlen(newuuid),
newe, NULL)) {
LOG("entry cache replace: can't add uuid\n", 0, 0, 0);
- remove_hash(cache->c_dntable, (void *)newndn, strlen(newndn));
- remove_hash(cache->c_idtable, &(newe->ep_id), sizeof(ID));
+ if(remove_hash(cache->c_dntable, (void *)newndn, strlen(newndn)) == 0){
+ LOG("entry cache replace: failed to remove dn table(uuid cache)\n", 0, 0, \
0); + }
+ if(remove_hash(cache->c_idtable, &(newe->ep_id), sizeof(ID)) == 0){
+ LOG("entry cache replace: failed to remove id table(uuid cache)\n", 0, 0, \
0); + }
PR_Unlock(cache->c_mutex);
return 1;
}
@@ -1357,7 +1363,9 @@ entrycache_add_int(struct cache *cache, struct backentry *e, \
int state, PR_Unlock(cache->c_mutex);
return 0;
}
- remove_hash(cache->c_dntable, (void *)ndn, strlen(ndn));
+ if(remove_hash(cache->c_dntable, (void *)ndn, strlen(ndn)) == 0){
+ LOG("entrycache_add_int: failed to remove dn table\n", 0, 0, 0);
+ }
e->ep_state |= ENTRY_STATE_NOTINCACHE;
PR_Unlock(cache->c_mutex);
return -1;
@@ -1369,8 +1377,12 @@ entrycache_add_int(struct cache *cache, struct backentry *e, \
int state, NULL)) {
LOG("entry %s already in uuid cache!\n", backentry_get_ndn(e),
0, 0);
- remove_hash(cache->c_dntable, (void *)ndn, strlen(ndn));
- remove_hash(cache->c_idtable, &(e->ep_id), sizeof(ID));
+ if(remove_hash(cache->c_dntable, (void *)ndn, strlen(ndn)) == 0){
+ LOG("entrycache_add_int: failed to remove dn table(uuid cache)\n", \
0, 0, 0); + }
+ if(remove_hash(cache->c_idtable, &(e->ep_id), sizeof(ID)) == 0){
+ LOG("entrycache_add_int: failed to remove id table(uuid cache)\n", \
0, 0, 0); + }
e->ep_state |= ENTRY_STATE_NOTINCACHE;
PR_Unlock(cache->c_mutex);
return -1;
diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c \
b/ldap/servers/slapd/back-ldbm/dblayer.c index 09d10a0..227218f 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -3884,7 +3884,7 @@ static int txn_test_threadmain(void *param)
txn_test_iter **ttilist = NULL;
size_t tticnt = 0;
DB_TXN *txn = NULL;
- txn_test_cfg cfg;
+ txn_test_cfg cfg = {0};
size_t counter = 0;
char keybuf[8192];
char databuf[8192];
@@ -3941,8 +3941,7 @@ wait_for_init:
object_release(inst_obj);
goto wait_for_init;
}
- dblayer_get_index_file(be, ai, &db, 0);
- if (NULL == db) {
+ if((dblayer_get_index_file(be, ai, &db, 0) != 0) || db == NULL){
if (strcasecmp(*idx, TXN_TEST_IDX_OK_IF_NULL)) {
object_release(inst_obj);
goto wait_for_init;
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c \
b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c index 449e02a..9b5b0de 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
@@ -2170,8 +2170,8 @@ _entryrdn_replace_suffix_id(DBC *cursor, DBT *key, DBT \
*adddata, char *realkeybuf = NULL;
DBT realkey;
char buffer[RDN_BULK_FETCH_BUFFER_SIZE];
- DBT data;
- DBT moddata;
+ DBT data = {0};
+ DBT moddata = {0};
rdn_elem **childelems = NULL;
rdn_elem **cep = NULL;
rdn_elem *childelem = NULL;
@@ -2216,7 +2216,6 @@ _entryrdn_replace_suffix_id(DBC *cursor, DBT *key, DBT \
*adddata, key->flags = DB_DBT_USERMEM;
/* Setting the bulk fetch buffer */
- memset(&data, 0, sizeof(data));
data.ulen = sizeof(buffer);
data.size = sizeof(buffer);
data.data = buffer;
@@ -2227,7 +2226,6 @@ _entryrdn_replace_suffix_id(DBC *cursor, DBT *key, DBT \
*adddata, realkey.size = realkey.ulen = strlen(realkeybuf) + 1;
realkey.flags = DB_DBT_USERMEM;
- memset(&moddata, 0, sizeof(moddata));
moddata.flags = DB_DBT_USERMEM;
for (db_retry = 0; db_retry < RETRY_TIMES; db_retry++) {
--
1.7.1
[Attachment #7 (text/plain)]
--
389-devel mailing list
389-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic