Blob Blame Raw
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