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