[Freeipa-devel] [PATCH 142] extdom: fix memory leak
Tomas Babej
tbabej at redhat.com
Tue Mar 10 11:14:53 UTC 2015
On 03/10/2015 12:10 PM, Sumit Bose wrote:
> On Tue, Mar 10, 2015 at 11:59:45AM +0100, Tomas Babej wrote:
>> On 03/05/2015 08:00 AM, Alexander Bokovoy wrote:
>>> On Wed, 04 Mar 2015, Nathan Kinder wrote:
>>>>
>>>> On 03/04/2015 10:34 PM, Alexander Bokovoy wrote:
>>>>> On Wed, 04 Mar 2015, Sumit Bose wrote:
>>>>>> Hi,
>>>>>>
>>>>>> while running 389ds with valgrind to see if my other patches
>>>>>> introduced
>>>>>> a memory leak I found an older one which is fixed by this patch.
>>>>>>
>>>>>> bye,
>>>>>> Sumit
>>>>> >From bb02cdc135fecc1766b17edd61554dbde9bccd0b Mon Sep 17 00:00:00 2001
>>>>>> From: Sumit Bose <sbose at redhat.com>
>>>>>> Date: Wed, 4 Mar 2015 17:53:08 +0100
>>>>>> Subject: [PATCH] extdom: fix memory leak
>>>>>>
>>>>>> ---
>>>>>> daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git
>>>>>> a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
>>>>>> b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
>>>>>> index
>>>>>> a040f2beba073d856053429face2f464347b2524..708d0e4a2fc9da4f87a24a49c945587049f7280f
>>>>>>
>>>>>> 100644
>>>>>> --- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
>>>>>> +++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
>>>>>> @@ -156,6 +156,7 @@ done:
>>>>>> LOG("%s", err_msg);
>>>>>> }
>>>>>> slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
>>>>>> + ber_bvfree(ret_val);
>>>>>> free_req_data(req);
>>>>>> return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
>>>>>> }
>>>>> I can see in 389-ds code that it actually tries to remove the value in
>>>>> the end of extended operation handling:
>>>> This below code snippet is freeing the extended operation request value
>>>> (SLAPI_EXT_OP_REQ_VALUE), not the return value (SLAPI_EXT_OP_RET_VAL).
>>>>
>>>> If you look at check_and_send_extended_result() in the 389-ds code,
>>>> you'll see where the extended operation return value is sent, and it
>>>> doesn't perform a free. It is up to the plug-in to perform the free. A
>>>> good example of this in the 389-ds code is in the passwd_modify_extop()
>>>> function.
>>>>
>>>>
>>>> Sumit's code looks good to me. ACK.
>>> Argh. Sorry for confusion of RET vs REQ. Morning, I need coffee!
>>>
>>> ACK.
>>>
>>>> -NGK
>>>>
>>>>> slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_OID, extoid );
>>>>> slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, &extval );
>>>>> slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT,
>>>>> &pb->pb_op->o_isroot);
>>>>>
>>>>> rc = plugin_call_exop_plugins( pb, extoid );
>>>>>
>>>>> if ( SLAPI_PLUGIN_EXTENDED_SENT_RESULT != rc ) {
>>>>> if ( SLAPI_PLUGIN_EXTENDED_NOT_HANDLED == rc ) {
>>>>> lderr = LDAP_PROTOCOL_ERROR; /* no plugin
>>>>> handled the op */
>>>>> errmsg = "unsupported extended operation";
>>>>> } else {
>>>>> errmsg = NULL;
>>>>> lderr = rc;
>>>>> }
>>>>> send_ldap_result( pb, lderr, NULL, errmsg, 0, NULL );
>>>>> }
>>>>> free_and_return:
>>>>> if (extoid)
>>>>> slapi_ch_free((void **)&extoid);
>>>>> if (extval.bv_val)
>>>>> slapi_ch_free((void **)&extval.bv_val);
>>>>> return;
>>>>>
>>>>>
>> The patch does not apply to the current master branch.
>>
>> Sumit, can you send a updated version?
> sure, new version attached.
>
> bye,
> Sumit
>
>> Thanks,
>> Tomas
Thanks,
Pushed to master: 8dac096ae3a294dc55b32b69b873013fd687e945
More information about the Freeipa-devel
mailing list