[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#8847) New LDAP URL syntax to support binding to specific IP address at client side



Thanks for the updated patch.

diff --git a/include/ldap.h b/include/ldap.h
index c58c576d3..78e07d49f 100644
--- a/include/ldap.h
+++ b/include/ldap.h
@@ -138,6 +138,7 @@ LDAP_BEGIN_DECL
 #define    LDAP_OPT_CONNECT_ASYNC      0x5010  /* create connections asynchronously */
 #define    LDAP_OPT_CONNECT_CB         0x5011  /* connection callbacks */
 #define    LDAP_OPT_SESSION_REFCNT     0x5012  /* session reference count */
+#define LDAP_OPT_SOCKET_BIND_ADDRESSES          0x5013  /* user configured bind IPs */

 /* OpenLDAP TLS options */
 #define LDAP_OPT_X_TLS             0x6000


You've got strange whitespace here. Please re-read the CONTRIBUTING docs; we use 4-column tab stops.


diff --git a/libraries/libldap/bind_ip.c b/libraries/libldap/bind_ip.c
new file mode 100644
index 000000000..56eddda43
--- /dev/null
+++ b/libraries/libldap/bind_ip.c
@@ -0,0 +1,102 @@
+/* LIBLDAP bind_ip.c*/
+/* $OpenLDAP$ */
+/* This work is part of OpenLDAP Software <http://www.openldap.org/>.
+ *


I see no reason why these functions are exposed in their own source file or exported from libldap.
parse_and_validate_sourceip() could just be a static function inside options.c

get_IPvX_from_lst() can just be statics in os-ip.c. In particular, you've defined these
as global but they don't have an ldap_ prefix, which is just wrong.

The work of parse_and_validate is wasted since you call inet_pton both for validating
and for using the result later. You should simply save the IPv4 and IPv6 binary addresses
when the validation succeeds. There should be no get_IPvX_from_lst() functions at all.

-- 
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/