On Sun, 2015-10-18 at 11:08 +0200, Thomas Faber wrote:
diff --git a/dlls/wldap32/winldap_private.h b/dlls/wldap32/winldap_private.h index a19ceb1..8596d1d 100644 --- a/dlls/wldap32/winldap_private.h +++ b/dlls/wldap32/winldap_private.h @@ -361,8 +361,8 @@ PCHAR * CDECL ldap_get_valuesA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PWCHAR * CDECL ldap_get_valuesW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); PBERVAL * CDECL ldap_get_values_lenA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PBERVAL * CDECL ldap_get_values_lenW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); -WLDAP32_LDAP * CDECL ldap_initA(const PCHAR,ULONG); -WLDAP32_LDAP * CDECL ldap_initW(const PWCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initA(PCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initW(PWCHAR,ULONG);
You should make the implementation match the prototype instead.
On 19 October 2015 at 10:09, Hans Leidekker hans@codeweavers.com wrote:
On Sun, 2015-10-18 at 11:08 +0200, Thomas Faber wrote:
diff --git a/dlls/wldap32/winldap_private.h b/dlls/wldap32/winldap_private.h index a19ceb1..8596d1d 100644 --- a/dlls/wldap32/winldap_private.h +++ b/dlls/wldap32/winldap_private.h @@ -361,8 +361,8 @@ PCHAR * CDECL ldap_get_valuesA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PWCHAR * CDECL ldap_get_valuesW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); PBERVAL * CDECL ldap_get_values_lenA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PBERVAL * CDECL ldap_get_values_lenW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); -WLDAP32_LDAP * CDECL ldap_initA(const PCHAR,ULONG); -WLDAP32_LDAP * CDECL ldap_initW(const PWCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initA(PCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initW(PWCHAR,ULONG);
You should make the implementation match the prototype instead.
The const does (almost) nothing. The intention may have been for these to be "const char *" and "const WCHAR *", but that's not how typedefs work. That's also one of the many reasons why pointer typedefs like PCHAR and PWCHAR are usually best avoided.
On Mon, 2015-10-19 at 11:28 +0200, Henri Verbeet wrote:
On 19 October 2015 at 10:09, Hans Leidekker hans@codeweavers.com wrote:
On Sun, 2015-10-18 at 11:08 +0200, Thomas Faber wrote:
diff --git a/dlls/wldap32/winldap_private.h b/dlls/wldap32/winldap_private.h index a19ceb1..8596d1d 100644 --- a/dlls/wldap32/winldap_private.h +++ b/dlls/wldap32/winldap_private.h @@ -361,8 +361,8 @@ PCHAR * CDECL ldap_get_valuesA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PWCHAR * CDECL ldap_get_valuesW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); PBERVAL * CDECL ldap_get_values_lenA(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PCHAR); PBERVAL * CDECL ldap_get_values_lenW(WLDAP32_LDAP*,WLDAP32_LDAPMessage*,PWCHAR); -WLDAP32_LDAP * CDECL ldap_initA(const PCHAR,ULONG); -WLDAP32_LDAP * CDECL ldap_initW(const PWCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initA(PCHAR,ULONG); +WLDAP32_LDAP * CDECL ldap_initW(PWCHAR,ULONG);
You should make the implementation match the prototype instead.
The const does (almost) nothing. The intention may have been for these to be "const char *" and "const WCHAR *", but that's not how typedefs work. That's also one of the many reasons why pointer typedefs like PCHAR and PWCHAR are usually best avoided.
I'm not sure that's a reason to differ from the psdk though. Note that these definitions are duplicated in winldap_private.h (with some modifications) because including the public header would cause conflicts with native ldap headers.
On 19 October 2015 at 11:54, Hans Leidekker hans@codeweavers.com wrote:
I'm not sure that's a reason to differ from the psdk though. Note that
I can't answer that for you, but personally I'm usually doing the reverse. I.e., only matching the PSDK when there's a good reason to. Usually that means either things that affect the ABI or things that affect the API in a way that would be visible to winelib applications.
On 2015-10-19 12:15, Henri Verbeet wrote:
On 19 October 2015 at 11:54, Hans Leidekker hans@codeweavers.com wrote:
I'm not sure that's a reason to differ from the psdk though. Note that
I can't answer that for you, but personally I'm usually doing the reverse. I.e., only matching the PSDK when there's a good reason to. Usually that means either things that affect the ABI or things that affect the API in a way that would be visible to winelib applications.
Thanks for your reviews! I do see that my patch created an inconsistency; sending a new version now. I'm going with adding the 'const' to the implementation now simply because it's the smaller change, I'm happy to remove it from both headers instead if that's the general (or Alexandre's) preference.
Thanks. -Thomas
-
Hans Leidekker -
Henri Verbeet -
Thomas Faber