On Tue, Nov 27, 2012 at 11:27:03PM -0500, Simo Sorce wrote:
On Wed, 2011-08-10 at 15:29 +0200, Jan Zelený wrote:
On Wed, Aug 10, 2011 at 10:14:14AM +0200, Jan Zelený wrote:
This patch should not be pushed to master, but I would like to get it reviewed anyway.
It should be used to provide a custom build for users experiencing cases where ldap_search_ext would block (c.f. https://bugzilla.redhat.com/show_bug.cgi?id=728343)
For example: export SSSD_DEBUG_LDAP_SEARCH="0xffff" would set LDAP_DEBUG_ANY
The attached patch applies cleanly on the RHEL6.1 branch. I also have a version that applies on master/1.5 if needed.
I have three things: On line 98 you close the original stderr - this is not a problem, but it is redundant
On line 162 you are using strtol and testing that it did not fail. Please consider two possibilities that you haven't considered in your patch:
- the function did not fail, but non-zero value remains in errno from
previous program run
- the env. variable SSSD_DEBUG_LDAP_SEARCH contains empty string (I'm not
sure whether it is possible)
Other than that the patch looks fine.
Jan
Thanks, a new patch is attached.
Looks ok now, Ack
It looks like this patch, although acked was never pushed upstream ? Has it been dropped offline ?
Simo.
-- Simo Sorce * Red Hat, Inc * New York
See this sentence in my original mail:
This patch should not be pushed to master, but I would like to get it reviewed anyway.
It should be used to provide a custom build for users experiencing cases where ldap_search_ext would block (c.f. https://bugzilla.redhat.com/show_bug.cgi?id=728343)
So no, it was never pushed to master. It provides a *lot* of debugging info and may easily flood the logs. We may want to reconsider the submission, but I would do so only if we dedicate one log level to low level tracing such as ldb transactions or libldb error output.