https://bugs.openldap.org/show_bug.cgi?id=8847
--- Comment #31 from Ryan Tandy <[email protected]> --- Hello, I have been reviewing and testing this patch and I think that there are a number of issues, some less severe and some more, that should still be addressed. In general the patch does not seem well adapted to the surrounding code. For example things have been added at random positions in lists that previously were sorted, and the whitespace style (and code style generally) are quite different from the existing code. Also, the new code does not seem to respect the configure option (and #ifdefs etc) for disabling IPv6 support. doc/man/man3/ldap_get_option.3: - LDAP_OPT_SOCKET_BIND_ADDRESSES added at the wrong place doc/man/man5/ldap.conf.5: - SOCKET_BIND_ADDRESSES added at the wrong place - typo (seperated -> separated) libraries/libldap/ldap-int.h: - /* pull in netinet/in */ is a useless comment - fails to compile under MinGW (there is no netinet/in.h header) -> I could be wrong but 'struct in_addr' feels rather low-level for this file? but I'm not sure what a better design would look like... - should not include IPv6 bits if IPv6 disabled - LDAP_LDO_NULLARG has not been updated (gcc generates a warning) - if ITS#6567 is finished before this one, MAX_LDAP_ADDR_LEN will probably need an update ("GSSAPI_ALLOW_REMOTE_PRINCIPAL" is longer than "SOCKET_BIND_ADDRESSES" is longer than "TLS_CIPHER_SUITE") libraries/libldap/options.c: - in ldap_set_option: other options reset to default when invalue == NULL, it would be nice if this would do the same - ldap_validate_and_fill_sourceip feels a bit weird again, there are no other similar functions in this file... maybe os-ip.c or util-int.c? - in the existing code, inet_pton is only used if LDAP_PF_INET6; should probably follow that pattern (there is also HAVE_INET_NTOP...) libraries/libldap/os-ip.c: - possibly the new code should be in ldap_int_prepare_socket()? not sure... - address family mismatch (only one bind address specified and socket uses the other family) ignored; should we try to catch it? -> MS implementation returns LDAP_SERVER_DOWN when this happens -- You are receiving this mail because: You are on the CC list for the bug.
