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

[hyc@symas.com]

Sharma, Ramakant 2. (Nokia - IN/Bangalore) wrote:
> Hi Howard,
> 
> Please provide your valuable inputs for the below patch.

It is a little strange that ldap_set_option() takes a string but ldap_get_option()
returns a binary structure. get/set should be totally symmetric. It would be
simplest to just save a copy of the string given to set and return the string in get.

In libldap/os-ip.c, you should not be using ldap_get_option() to retrieve the values.
You're operating inside libldap, you can access the structure directly. In particular,
it's a waste to use ldap_get_option here and create a dynamically allocated copy of the
address struct. Look at how the existing code works. Notice that it just uses
ld->ld_options directly.

This is rule #1 in software development - when extending existing code, your new code
should do things the same way existing code does. Corollaries to this rule are:
don't patch code without reading it first, and don't patch code without studying
its complete context.


> 
> Also let us know how to proceed further.
> 
> BR,
> Ramakant Sharma
> 
> -----Original Message-----
> From: Sharma, Ramakant 2. (Nokia - IN/Bangalore) 
> Sent: Monday, January 28, 2019 10:10 AM
> To: Howard Chu <hyc@symas.com>; openldap-its@OpenLDAP.org
> Cc: Singam, Sudhir (Nokia - IN/Bangalore) <sudhir.singam@nokia.com>
> Subject: RE: (ITS#8847) New LDAP URL syntax to support binding to specific IP address at client side
> 
> Hi Howard,
> 
> I have uploaded the new patch which addresses the previous comments.
> 
> Kindly check and provide your valuable comments .
> 
> ftp://ftp.openldap.org/incoming/ramakant-sharma-190121.patch
> 
> BR,
> Ramakant Sharma
> 


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