Kurt Zeilenga wrote: > Doug, > > I note as a general rule, which I will follow in this case, I don't > review patches authors would like included in OpenLDAP Software until they have been formally submitted for review. While the ITS submission provides for a review of what was done, my purpose in directing the conversation here is to discuss why. It's not clear to me that this old draft is still viable or desirable. As

On Aug 18, 2010, at 4:34 PM, Howard Chu wrote: > my purpose in directing the conversation here is to discuss why. It's not clear to me that this old draft is still viable or desirable. As we've talked about many times before, the C API is a huge mess and we need to toss the current one out and go back to the drawing board, preferably to come up with a new API that doesn't meld operational behavior (e.g. referral chasing) with the low level protocol implementation, and one that is largely free-threaded. At one time, I must have through the draft's suggested changes to the LDAPext-proposed U-Mich-based LDAP C API where a good idea. Lots of time has passed since that time. Taking a step back, as I'ved noted many times, I would support introduction of a new LDAP C API which focused on encoding/decoding of LDAP constructions and not I/O management and not higher level handling such as managing distributed contexts. Such a API could and, I certainly would argue, should be be thread-free. Given this, one could use this to offer higher level APIs. I would prefer to, in the long run, dump the U-Mich APIs as I think it's a dead-end (though I think that dead-end might be many, many miles down the road). I don't object to continued work on the OpenLDAP LDAP C API. Bug fixes (Ando commented that some of the Doug's changes can be viewed as Bug fixes) are certainly not objectionable. I also don't object to introduction of additional capability so long as that introduction doesn't "break" applications specifically designed to use prior (within reason) versions of the OpenLDAP LDAP C API upon upgrade. I am generally opposed to adding additional APIs just to additional APIs which offer zero new capability and am generally opposed to adding new deprecated APIs. Also, I also don't generally accept the argument that introduction of interfaces found in other LDAP C APIs 'eases' porting of LDAP applications. But let's look at ldap_get_lderrno() specifically. Today there are multiple flavors of the ldap_get_lderrno() API. I found two different non-OpenLDAP LDAP C APIs to include an ldap_get_lderrno() interface with the signature: int ldap_get_lderrno(LDAP *ld, char **dn, char **errmsg); but with incompatible semantics. One required the caller to free the strings returned in dn and errmsg. One didn't, as the dn and errmsg were simply set to point to the strings held in the LDAP structure. The expected problems introduced are differ depending on whether OpenLDAP ldap_get_lderrno(), it should be clear that problems are introduced. Simply put, despite the introduction of ldap_get_lderrno(), application developers need to be careful in porting to ensure that OpenLDAP offers compatible semantics to what they offer. And that likely requires reading, at a minimum, the documentation. I rather developers simply port their code to use ldap_get_option(3) interface here, which because of inclusion in the LDAPEXT LDAP C API drafts (even through expired), are far more consistently implemented than routines such as ldap_get_lderrno(). I also oppose adding ldap_get_lderror(). Beyond this, I haven't reviewed the patch in detail. I think I'll leave that more current and more active in the development of OpenLDAP Software. -- Kurt

I would like to propose a patch to the OpenLDAP C APIs to support the operation thread safe features described in the draft-zeilenga-ldap-c-api-concurrency drafts. The enhancements add upwardly compatible interfaces and should have no behavioral regressions w.r.t. any existing libldap interfaces. The new concurrency APIs include: ldap_dup and ldap_destroy They are described in more detail in the concurrency draft and in the applicable man paged. The related feature options are also provided. Additionally I would like to propose 1 new public error API ldap_get_lderror which returns all three ldap_get_option results: LDAP_OPT_MATCHED_DN LDAP_OPT_DIAGNOSTIC_MESSAGE LDAP_OPT_RESULT_CODE in a single API, and 1 new depreciated public API: ldap_get_lderrno which performs almost the same function as ldap_get_error but does so in a mt-unsafe manner, but that is compatible with Mozilla's libldap API of the same name. The point of the two new error APIs is to provide a transition path from Mozilla libldap users that still use ldap_get_lderrno to either a similar API that obeys OpenLDAP like practices (ldap_get_error) or as a transition until any existing code can be converted away from ldap_get_lderrno to just using ldap_get_options as needed. The patch includes a new test case to test the new functionality and man pages. The new test case needs additional work for some of the threading models. Any input here is requested, since I do not have testing resources for all the threading combination and do not have the ability to test the Windows model. I am looking forward to any and all comments. I plan to submit a formal ITS with a finalized version of changes once any and all issues are thrashed out and resolved to a level that they are acceptable for integration. Just for the record: This patch file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Oracle Corporation.Oracle Corporation has not assigned rights and/or interest in this work to any party. I, Douglas Leavitt am authorized by Oracle Corporation, my employer, to release this work under the following terms. The initial version of my proposed patch can be found here: http://cr.opensolaris.org/~djl/openldap-codereview This includes a diff -U of the full patch (/diffU) a webrev diff (/webrev) for on-line review and a tar ball image of the OpenLDAP HEAD as of 8/13/10 with the current patch applied. Thank you for you consideration, and I look forward to any and all comments. Doug Leavitt