From b67e23018d0be5de44e709d2cc7c074b42aae586 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Tue, 25 Jun 2013 10:38:26 -0400
Subject: [PATCH 38/39] Coverity Fixes
13177 - Unreachable code (backend.c)
13175 - Resource leak (repl5_ruv.c)
Did code cleanup in libaccess/oneval.cpp before I do the fix for 13176(resource leak).
This is will be a separate commit.
https://bugzilla.redhat.com/show_bug.cgi?id=970221
Reviewed by: richm(Thanks!)
(cherry picked from commit 6e07f4df6c1963f40368d0ae17e0775aa33362af)
removed the repl5_ruv.c changes because they do not apply to 1.3.1
(cherry picked from commit 21dccd37e7883ff3b9ace01350e3123dc42b3b82)
---
ldap/servers/slapd/backend.c | 1 -
lib/libaccess/oneeval.cpp | 259 +++++++++++++++++++++---------------------
2 files changed, 128 insertions(+), 132 deletions(-)
diff --git a/ldap/servers/slapd/backend.c b/ldap/servers/slapd/backend.c
index ead251e..8a72b13 100644
--- a/ldap/servers/slapd/backend.c
+++ b/ldap/servers/slapd/backend.c
@@ -669,7 +669,6 @@ slapi_back_transaction_commit(Slapi_PBlock *pb)
} else {
return txn_commit(pb);
}
- return txn_commit(pb);
}
/* API to expose DB transaction abort */
diff --git a/lib/libaccess/oneeval.cpp b/lib/libaccess/oneeval.cpp
index fbaa6d8..8077969 100644
--- a/lib/libaccess/oneeval.cpp
+++ b/lib/libaccess/oneeval.cpp
@@ -358,8 +358,8 @@ ACLEvalBuildContext(
/* Allocate the cache context and link it into the ACLListHandle */
cache = (ACLListCache_t *)PERM_CALLOC(sizeof(ACLListCache_t));
if (cache == NULL) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4010, ACL_Program, 0);
- goto error;
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4010, ACL_Program, 0);
+ goto error;
}
/* Allocate the access rights hash table */
@@ -371,9 +371,9 @@ ACLEvalBuildContext(
NULL);
if (cache->Table == NULL) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4000, ACL_Program, 1,
- XP_GetAdminStr(DBT_EvalBuildContextUnableToCreateHash));
- goto error;
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4000, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToCreateHash));
+ goto error;
}
wrapper = acleval->acllist->acl_list_head;
@@ -395,162 +395,159 @@ ACLEvalBuildContext(
XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAceEntry));
goto error;
}
- new_ace->acep = ace;
+ new_ace->acep = ace;
ace_cnt++;
if (cache->acelist == NULL)
- cache->acelist = acelast = new_ace;
+ cache->acelist = acelast = new_ace;
else {
if(acelast)
- acelast->next = new_ace;
- acelast = new_ace;
- new_ace->acep = ace;
+ acelast->next = new_ace;
+ acelast = new_ace;
+ new_ace->acep = ace;
}
- new_ace->next = NULL;
+ new_ace->next = NULL;
- argp = ace->expr_argv;
+ argp = ace->expr_argv;
- switch (ace->expr_type)
- {
- case ACL_EXPR_TYPE_ALLOW:
- case ACL_EXPR_TYPE_DENY:
-
- /* Add this ACE to the appropriate entries in the access rights
- * hash table
- */
- while (*argp)
- {
- entry =
- (ACLAceNumEntry_t *)PERM_CALLOC(sizeof(ACLAceNumEntry_t));
- if (entry == (ACLAceNumEntry_t *)NULL) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4030, ACL_Program, 1,
- XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAceEntry));
- goto error;
- }
- if (cache->chain_head == NULL)
- cache->chain_head = cache->chain_tail = entry;
- else {
- cache->chain_tail->chain = entry;
- cache->chain_tail = entry;
+ switch (ace->expr_type)
+ {
+ case ACL_EXPR_TYPE_ALLOW:
+ case ACL_EXPR_TYPE_DENY:
+
+ /* Add this ACE to the appropriate entries in the access rights
+ * hash table
+ */
+ while (*argp)
+ {
+ entry = (ACLAceNumEntry_t *)PERM_CALLOC(sizeof(ACLAceNumEntry_t));
+ if (entry == (ACLAceNumEntry_t *)NULL) {
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4030, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAceEntry));
+ goto error;
+ }
+ if (cache->chain_head == NULL)
+ cache->chain_head = cache->chain_tail = entry;
+ else {
+ cache->chain_tail->chain = entry;
+ cache->chain_tail = entry;
+ }
+ entry->acenum = ace_cnt;
+
+ /*
+ * OK to call PL_HasTableLookup() even though it mods
+ * the Table as this routine is called in critical section.
+ */
+ temp_entry = (ACLAceNumEntry_t *)PL_HashTableLookup(cache->Table, *argp);
+ /* the first ACE for this right? */
+ if (temp_entry) {
+ /* Link it in at the end */
+ while (temp_entry->next) {
+ temp_entry = temp_entry->next;
+ }
+ temp_entry->next = entry;
+ } else /* just link it in */
+ PR_HashTableAdd(cache->Table, *argp, entry);
+ argp++;
}
- entry->acenum = ace_cnt;
-
- /*
- * OK to call PL_HasTableLookup() even though it mods
- * the Table as this routine is called in critical section.
- */
- temp_entry = (ACLAceNumEntry_t *)PL_HashTableLookup(cache->Table, *argp);
- /* the first ACE for this right? */
- if (temp_entry) {
- /* Link it in at the end */
- while (temp_entry->next) {
- temp_entry = temp_entry->next;
- }
- temp_entry->next = entry;
- } else /* just link it in */
- PR_HashTableAdd(cache->Table, *argp, entry);
-
- argp++;
- }
-
- /* See if any of the clauses require authentication. */
- if (curauthplist) {
- for (i = 0; i < ace->expr_term_index; i++) {
- expr = &ace->expr_arry[i];
- rv = PListFindValue(curauthplist, expr->attr_name,
- NULL, &authplist);
- if (rv > 0) {
- /* First one for this ACE? */
- if (!new_ace->autharray) {
- new_ace->autharray = (PList_t *)PERM_CALLOC(sizeof(PList_t) * ace->expr_term_index);
+ /* See if any of the clauses require authentication. */
+ if (curauthplist) {
+ for (i = 0; i < ace->expr_term_index; i++) {
+ expr = &ace->expr_arry[i];
+ rv = PListFindValue(curauthplist, expr->attr_name,
+ NULL, &authplist);
+ if (rv > 0) {
+ /* First one for this ACE? */
if (!new_ace->autharray) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4040, ACL_Program, 1, XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPointerArray));
- goto error;
- }
- }
- new_ace->autharray[i] = authplist;
- }
- }
- }
- break;
-
- case ACL_EXPR_TYPE_AUTH:
+ new_ace->autharray = (PList_t *)PERM_CALLOC(sizeof(PList_t) * ace->expr_term_index);
+ if (!new_ace->autharray) {
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4040, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPointerArray));
+ goto error;
+ }
+ }
+ new_ace->autharray[i] = authplist;
+ }
+ }
+ }
+ break;
- /* Allocate the running auth tables if none yet */
- if (!curauthplist) {
- curauthplist = PListNew(NULL);
- if (!curauthplist) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1, XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
- goto error;
- }
- absauthplist = PListNew(NULL);
- if (!absauthplist) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1, XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
- goto error;
- }
- } else { /* duplicate the existing auth table */
- curauthplist = PListDuplicate(curauthplist, NULL, 0);
- if (!curauthplist) {
- nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1, XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
- goto error;
- }
- }
+ case ACL_EXPR_TYPE_AUTH:
+
+ /* Allocate the running auth tables if none yet */
+ if (!curauthplist) {
+ curauthplist = PListNew(NULL);
+ if (!curauthplist) {
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
+ goto error;
+ }
+ absauthplist = PListNew(NULL);
+ if (!absauthplist) {
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
+ goto error;
+ }
+ } else { /* duplicate the existing auth table */
+ curauthplist = PListDuplicate(curauthplist, NULL, 0);
+ if (!curauthplist) {
+ nserrGenerate(errp, ACLERRNOMEM, ACLERR4050, ACL_Program, 1,
+ XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAuthPlist));
+ goto error;
+ }
+ }
- /* For each listed attribute */
- while (*argp)
- {
- /* skip any attributes that were absoluted */
- if (PListFindValue(absauthplist, *argp, NULL, NULL) < 0)
- {
- /* Save pointer to the property list */
- PListInitProp(curauthplist, 0, *argp, ace->expr_auth,
- ace->expr_auth);
- if (IS_ABSOLUTE(ace->expr_flags))
- PListInitProp(absauthplist, 0, *argp, NULL,
- NULL);
- }
+ /* For each listed attribute */
+ while (*argp) {
+ /* skip any attributes that were absoluted */
+ if (PListFindValue(absauthplist, *argp, NULL, NULL) < 0){
+ /* Save pointer to the property list */
+ PListInitProp(curauthplist, 0, *argp, ace->expr_auth, ace->expr_auth);
+ if (IS_ABSOLUTE(ace->expr_flags))
+ PListInitProp(absauthplist, 0, *argp, NULL, NULL);
+ }
+ argp++;
+ }
- argp++;
- }
+ break;
- break;
+ case ACL_EXPR_TYPE_RESPONSE:
+ (void) ACL_ExprGetDenyWith(NULL, ace, &cache->deny_type,
+ &cache->deny_response);
+ break;
- case ACL_EXPR_TYPE_RESPONSE:
- (void) ACL_ExprGetDenyWith(NULL, ace, &cache->deny_type,
- &cache->deny_response);
- break;
+ default:
+ PR_ASSERT(0);
- default:
- PR_ASSERT(0);
-
- } /* switch expr_type */
+ } /* switch expr_type */
- new_ace->global_auth = curauthplist;
- ace = ace->expr_next;
- }
+ new_ace->global_auth = curauthplist;
+ ace = ace->expr_next;
+ } /* while (ace) */
- /* Next ACL please */
+ /* Next ACL please */
wrapper = wrapper->wrap_next;
}
if (absauthplist)
- PListDestroy(absauthplist);
+ PListDestroy(absauthplist);
/* This must be done last to avoid a race in initialization */
- acleval->acllist->cache = (void *)cache;
+ acleval->acllist->cache = (void *)cache;
return 0;
error:
if (curauthplist)
- PListDestroy(curauthplist);
+ PListDestroy(curauthplist);
if (absauthplist)
- PListDestroy(absauthplist);
+ PListDestroy(absauthplist);
if (cache) {
ACL_EvalDestroyContext(cache);
}
acleval->acllist->cache = NULL;
+
return ACL_RES_ERROR;
}
--
1.7.1