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. > > Why all this copying around ? > > It should be sufficient to call krb5_free_data_contents() at the end at > > most. > > You are right, I overengineered this one. Fixed. Much better now, thanks. > Attaching a patch fixing this one + an issue found by Sumit (sigh, > quick-fix-before-send type of error). Looks good to me now. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
