[Freeipa-devel] [PATCH] 370-371 ipa-kdb: remove memory leaks
Martin Kosek
mkosek at redhat.com
Thu Feb 14 09:16:44 UTC 2013
On 02/14/2013 10:07 AM, Sumit Bose wrote:
> On Tue, Feb 12, 2013 at 04:42:52PM +0100, Martin Kosek wrote:
>> On 02/12/2013 04:26 PM, Simo Sorce wrote:
>>> On Tue, 2013-02-12 at 16:14 +0100, Martin Kosek wrote:
>>>> Explained in the commit description - this may not be super-critical, I just
>>>> followed info in ldap_search_ext() man page:
>>>>
>>>> ...
>>>>
>>>> Note that res parameter of ldap_search_ext_s() and ldap_search_s()
>>>> should be freed with
>>>> ldap_msgfree() regardless of return value of these functions.
>>>> ...
>>>
>>> OK.
>>>
>>>>> This snippet seem to change the logic.
>>>>>
>>>>> Before the condition was !ipadb_need_retry() and no you change it to
>>>>> ipadb_need_retry() so it looks reversed, was this on purpose ?
>>>>
>>>> As noted in commit description, this check was wrong and it was causing the
>>>> function to _always_ retry at least once because ipadb_need_retry returns true
>>>> when we need to retry and not 0 -> thus no "!" is needed.
>>>
>>> I do not like much 'sneaking' this kind of change in the same patch that
>>> fixes memory leaks, so if you want to plit in 2 patches it would be
>>> nice, but I am of course ok with the change.
>>
>> I agree with that. Lets do it right as we have not pushed it yet. I moved this
>> change to a separate patch (attached).
>
> I didn't found any issues while testing, so ACK.
>
> When I run kinit in a endless loop I still see a constant increase in
> the resident memory size, but it looks like it is not related to the
> ipadb code. I will investigate further.
>
> bye,
> Sumit
>
Ok, thanks. Pushed to master, ipa-3-1.
Martin
More information about the Freeipa-devel
mailing list