[PATCH 0/1] MR7335: wldap32: Build libldap with debugging facilities turned on, enable tracing with +libldap channel.
From: Dmitry Timoshkov dmitry@baikal.ru
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/wldap32/bind.c | 5 ++++- dlls/wldap32/init.c | 6 ++++++ dlls/wldap32/main.c | 7 +++++++ libs/ldap/Makefile.in | 2 +- 4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/dlls/wldap32/bind.c b/dlls/wldap32/bind.c index c297c234ad6..419dbb979ff 100644 --- a/dlls/wldap32/bind.c +++ b/dlls/wldap32/bind.c @@ -29,6 +29,7 @@ #include "winldap_private.h"
WINE_DEFAULT_DEBUG_CHANNEL(wldap32); +WINE_DECLARE_DEBUG_CHANNEL(libldap);
/*********************************************************************** * ldap_bindA (WLDAP32.@) @@ -200,6 +201,7 @@ ULONG CDECL ldap_bind_sW( LDAP *ld, WCHAR *dn, WCHAR *cred, ULONG method ) else if (method == WLDAP32_LDAP_AUTH_NEGOTIATE) { SEC_WINNT_AUTH_IDENTITY_W *id = (SEC_WINNT_AUTH_IDENTITY_W *)cred, idW; + unsigned int flags;
if (id && (id->Flags & SEC_WINNT_AUTH_IDENTITY_ANSI)) { @@ -209,7 +211,8 @@ ULONG CDECL ldap_bind_sW( LDAP *ld, WCHAR *dn, WCHAR *cred, ULONG method ) id = &idW; }
- ret = map_error( ldap_sasl_interactive_bind_s( CTX(ld), NULL, NULL, NULL, NULL, LDAP_SASL_QUIET, + flags = TRACE_ON( libldap ) ? 0 : LDAP_SASL_QUIET; + ret = map_error( ldap_sasl_interactive_bind_s( CTX(ld), NULL, NULL, NULL, NULL, flags, interact_callback, id ) ); if (id && (id->Flags & SEC_WINNT_AUTH_IDENTITY_ANSI)) { diff --git a/dlls/wldap32/init.c b/dlls/wldap32/init.c index 472bf2ffdbf..0e41e610458 100644 --- a/dlls/wldap32/init.c +++ b/dlls/wldap32/init.c @@ -34,6 +34,7 @@ #include "winldap_private.h"
WINE_DEFAULT_DEBUG_CHANNEL(wldap32); +WINE_DECLARE_DEBUG_CHANNEL(libldap);
/* Split a space separated string of hostnames into a string array */ static char **split_hostnames( const char *hostnames ) @@ -203,6 +204,11 @@ static LDAP *create_context( const char *url ) if (map_error( ldap_initialize( &CTX(ld), url ) ) == WLDAP32_LDAP_SUCCESS) { ldap_set_option( CTX(ld), WLDAP32_LDAP_OPT_PROTOCOL_VERSION, &version ); + if (TRACE_ON( libldap )) + { + int ld_debug = -1; + ldap_set_option( CTX(ld), LDAP_OPT_DEBUG_LEVEL, &ld_debug ); + } return ld; } free( ld ); diff --git a/dlls/wldap32/main.c b/dlls/wldap32/main.c index ac983372da7..0b79635b63d 100644 --- a/dlls/wldap32/main.c +++ b/dlls/wldap32/main.c @@ -24,10 +24,12 @@ #include "winbase.h"
#include "wine/debug.h" +#include "winldap_private.h"
HINSTANCE hwldap32;
WINE_DEFAULT_DEBUG_CHANNEL(wldap32); +WINE_DECLARE_DEBUG_CHANNEL(libldap);
BOOL WINAPI DllMain( HINSTANCE hinst, DWORD reason, LPVOID reserved ) { @@ -38,6 +40,11 @@ BOOL WINAPI DllMain( HINSTANCE hinst, DWORD reason, LPVOID reserved ) case DLL_PROCESS_ATTACH: hwldap32 = hinst; DisableThreadLibraryCalls( hinst ); + if (TRACE_ON( libldap )) + { + int ld_debug = -1; + ldap_set_option( NULL, LDAP_OPT_DEBUG_LEVEL, &ld_debug ); + } break; } return TRUE; diff --git a/libs/ldap/Makefile.in b/libs/ldap/Makefile.in index 5e5882584d0..8cfad386285 100644 --- a/libs/ldap/Makefile.in +++ b/libs/ldap/Makefile.in @@ -1,6 +1,6 @@ EXTLIB = libldap.a EXTRAINCL = -I$(srcdir)/include -EXTRADEFS = -D_TIMEVAL_DEFINED +EXTRADEFS = -D_TIMEVAL_DEFINED -DLDAP_DEBUG -DOLD_DEBUG
SOURCES = \ liblber/bprint.c \
If we want to do debug builds of libldap I think it should be off by default. It has side effects like enabling asserts. Why do we need a separate debug channel?
On Fri Feb 14 11:44:52 2025 +0000, Hans Leidekker wrote:
If we want to do debug builds of libldap I think it should be off by default. It has side effects like enabling asserts. Why do we need a separate debug channel?
Adding libldap output to wldap32 channel makes +wldap32 very massive and hard to read. I didn't see any asserts during GSS-SPNEGO debugging, and having this patch in a local build for an year++ didn't reveal anything like that. Contrary, it helped a lot with debugging wldap32 + kerberos pair, so I find it very helpful feature.
How would you propose making debug build off by default, if that's a desirable way to proceed?
On Fri Feb 14 11:44:52 2025 +0000, Dmitry Timoshkov wrote:
Adding libldap output to wldap32 channel makes +wldap32 very massive and hard to read. I didn't see any asserts during GSS-SPNEGO debugging, and having this patch in a local build for an year++ didn't reveal anything like that. Contrary, it helped a lot with debugging wldap32 + kerberos pair, so I find it very helpful feature. How would you propose making debug build off by default, if that's a desirable way to proceed?
Not sure how that should be done but I have used debugging builds of libdap and I don't think it's too much to ask the developer to enable it. It's also not common that we need to provide a user with a debug build so I'm not sure this is worth it.
On Fri Feb 14 12:19:23 2025 +0000, Hans Leidekker wrote:
Not sure how that should be done but I have used debugging builds of libdap and I don't think it's too much to ask the developer to enable it. It's also not common that we need to provide a user with a debug build so I'm not sure this is worth it.
How about dropping the libldap part of the patch and just leave the wldap32 one?
On Fri Feb 14 12:24:34 2025 +0000, Dmitry Timoshkov wrote:
How about dropping the libldap part of the patch and just leave the wldap32 one?
I think that's fine but why do you set the option both globally and on the context? Is it necessary to change the flags to ldap_sasl_interactive_bind_s()? I still think it should use the existing debug channel.
On Fri Feb 14 12:38:51 2025 +0000, Hans Leidekker wrote:
I think that's fine but why do you set the option both globally and on the context? Is it necessary to change the flags to ldap_sasl_interactive_bind_s()? I still think it should use the existing debug channel.
I can leave setting the LDAP_OPT_DEBUG_LEVEL in a single place. Which one would you prefer better?
Iit's very helpful to make ldap_sasl_interactive_bind_s() not quite.
Not a problem, I'll use existing debug channel.
On Fri Feb 14 12:47:02 2025 +0000, Dmitry Timoshkov wrote:
I can leave setting the LDAP_OPT_DEBUG_LEVEL in a single place. Which one would you prefer better? Iit's very helpful to make ldap_sasl_interactive_bind_s() not quite. Not a problem, I'll use existing debug channel.
I looked at libs/ldap source and turning on LDAP_DEBUG doesn't affect asserts at all, assert() statements verify the state in internal helpers.
On Fri Feb 14 12:51:41 2025 +0000, Dmitry Timoshkov wrote:
I looked at libs/ldap source and turning on LDAP_DEBUG doesn't affect asserts at all, assert() statements verify the state in internal helpers.
I'd say globally so you can get debug output even before a context is created. 0 == LDAP_SASL_AUTOMATIC which means it can prompt. So it changes behavior which the user may not expect.
On Fri Feb 14 12:59:02 2025 +0000, Hans Leidekker wrote:
I'd say globally so you can get debug output even before a context is created. 0 == LDAP_SASL_AUTOMATIC which means it can prompt. So it changes behavior which the user may not expect.
Wht do you mean by prompt? As far as I can see without LDAP_SASL_QUITE libldap calls sasl_getprop() which is part of our backend (sasl_w.c), and it never interactively propmps the user.
On Fri Feb 14 13:14:09 2025 +0000, Dmitry Timoshkov wrote:
Wht do you mean by prompt? As far as I can see without LDAP_SASL_QUITE libldap calls sasl_getprop() which is part of our backend (sasl_w.c), and it never interactively propmps the user.
From ldap.h: ``` /* Interaction flags (should be passed about in a control) * Automatic (default): use defaults, prompt otherwise * Interactive: prompt always * Quiet: never prompt */ #define LDAP_SASL_AUTOMATIC»»···0U #define LDAP_SASL_INTERACTIVE»··1U #define LDAP_SASL_QUIET»»···»···2U ```
On Fri Feb 14 13:26:44 2025 +0000, Hans Leidekker wrote:
From ldap.h:
/* Interaction flags (should be passed about in a control) * Automatic (default): use defaults, prompt otherwise * Interactive: prompt always * Quiet: never prompt */ #define LDAP_SASL_AUTOMATIC»»···0U #define LDAP_SASL_INTERACTIVE»··1U #define LDAP_SASL_QUIET»»···»···2U
Grepping for LDAP_SASL_AUTOMATIC and LDAP_SASL_INTERACTIVE over the libs/ldap sources yields nothing. So the only meaningful flag is LDAP_SASL_QUITE which turns off calling the backend, and our backend never propmpts the user.
Our backend is not complete. We shouldn't depend on the flag not being implemented.
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Hans Leidekker (＠hans)