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!)

1 files changed, 13 insertions, 4 deletions

diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c
index a9cd37ee..d6909ac8 100644
--- a/ldap/servers/slapd/valueset.c
+++ b/ldap/servers/slapd/valueset.c
@@ -1345,10 +1345,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.
      */
@@ -1368,8 +1364,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;
 }