[Freeipa-devel] [PATCH] 370-371 ipa-kdb: remove memory leaks

[Martin Kosek]

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

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-370-3-ipa-kdb-remove-memory-leaks.patch
Type: text/x-patch
Size: 5906 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130212/5307de92/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-371-ipa-kdb-fix-retry-logic-in-ipadb_deref_search.patch
Type: text/x-patch
Size: 1221 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130212/5307de92/attachment-0001.bin>