[389-commits] Branch 'Directory_Server_8_2_Branch' - ldap/servers

[Richard Allen Megginson]

 ldap/servers/slapd/valueset.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

New commits:
commit 0e888ff5f228fc7f5a05ba5061c57b6ba8f2ce16
Author: Rich Megginson <rmeggins at redhat.com>
Date:   Wed Feb 24 09:48:36 2010 -0700

    fix memory leak in attr replace when replacement fails
    
    if replacement of the attribute values fails (e.g. due to duplicate values)
    the valstoreplace is not freed - the caller expects the valueset_replace
    function to own the values passed in.  The function will now free the values
    if there was an error
    In addition, valueset_replace should not free the old values in case
    of error - it should leave the old values in the attribute
    Reviewed by: nhosoi (Thanks!)
    (cherry picked from commit 43894ffddf76baa4824c93d702ad4712e82b5b4e)

diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c
index 7334a7a..8aa33c5 100644
--- a/ldap/servers/slapd/valueset.c
+++ b/ldap/servers/slapd/valueset.c
@@ -1346,10 +1346,6 @@ valueset_replace(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value **valstoreplace)
 {
     int rc = LDAP_SUCCESS;
     int numberofvalstoreplace= valuearray_count(valstoreplace);
-    if(!valuearray_isempty(vs->va))
-    {
-        slapi_valueset_done(vs);
-    }
     /* verify the given values are not duplicated.
        if replacing with one value, no need to check.  just replace it.
      */
@@ -1369,8 +1365,21 @@ valueset_replace(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value **valstoreplace)
 
     if ( rc == LDAP_SUCCESS )
     {
+        /* values look good - replace the values in the attribute */
+        if(!valuearray_isempty(vs->va))
+        {
+            /* remove old values */
+            slapi_valueset_done(vs);
+        }
+        /* we now own valstoreplace */
         vs->va = valstoreplace;
     }
+    else
+    {
+        /* caller expects us to own valstoreplace - since we cannot
+           use them, just delete them */
+        valuearray_free(&valstoreplace);
+    }
     return rc;
 }