On Wed, Nov 28, 2012 at 07:40:32AM -0500, Stephen Gallagher wrote:
On Wed 28 Nov 2012 02:46:43 AM EST, Jakub Hrozek wrote:
>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.
That is supposed to be the point of SSSDBG_TRACE_ALL. I realize
we're not always consistent on that, though.
OK, attached is the original acked patch, just rebased on master.
Right now, the debug is controlled by a special environment variable. If
we were to use the patch upstream, I would propose just turning the
libldap debug based on an the debug_level option value.