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.