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 <[email protected]>; [email protected] > Cc: Singam, Sudhir (Nokia - IN/Bangalore) <[email protected]> > 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/
