diff options
author | Rich Megginson <rmeggins@redhat.com> | 2010-02-24 09:48:36 -0700 |
---|---|---|
committer | Rich Megginson <rmeggins@redhat.com> | 2010-02-24 11:09:07 -0700 |
commit | 43894ffddf76baa4824c93d702ad4712e82b5b4e (patch) | |
tree | d2fa795013e3fe0167d0e632173d4ec7fb6df90b /ldap | |
parent | c0fd0171fed64b026bc80bad872b6641a0c4d86f (diff) | |
download | ds-43894ffddf76baa4824c93d702ad4712e82b5b4e.tar.gz ds-43894ffddf76baa4824c93d702ad4712e82b5b4e.tar.xz ds-43894ffddf76baa4824c93d702ad4712e82b5b4e.zip |
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!)
Diffstat (limited to 'ldap')
-rw-r--r-- | ldap/servers/slapd/valueset.c | 17 |
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; } |