New LDAP C API

[Howard Chu]

Michael B Allen wrote:
> I just went through a lot of LDAP API related work so FWIW I think I
> can add some comments about this.
>   

This is great stuff. Probably should get added to the wiki.
> One thing that we did that I would really like to see in a new and
> improved API is allowing the user to set "attribute definitions". In
> our API we have a function with the following prototype:
>
>   int ldapx_set_attrdef(struct ldapx *lx,
>           const unsigned char *name,
>           int type,
>           uint32_t flags,
>           int conv);
>
> By default everything is multi-valued binary just like the ldap_* API but
> if you set an attribute definition you can specify that it's a single
> valued string or an integer or whatever. This allows applications to
> do things like convert a string from UTF-8 to the locale encoding and
> represent the element as a single valued element.
>   

This is clearly a higher level of functionality. I like it; I think we 
need to bring more actual schema-awareness into the client side. 
Currently there's nothing at all.
> But something even more useful about being able to set attribute
> definitions is allowing setting a "conversion sepcifier". For example,
> if we do the following:
>
>   ldapx_set_attrdef(lx,
>           "lastLogon",
>           LDAPX_TYPE_INT64,
>           LDAPX_SINGLE_VALUED,
>           LDAPX_CONV_TIME1970M_X_TIME1601);
>
> then the LDAPX_CONV_TIME1970M_X_TIME1601 means that at the API level
> all instances of lastLogon will be represeted as milliseconds since 1970
> UTC but at the storage level (in the DIT) they are nanos since 1601.
>   

Sounds good. Then of course there  needs to be an API for registering 
new conversion handlers.

The handling of integers has always bothered me too, but that's less of 
an API issue and more of a schema issue. The ASN.1 definition of 
integers is inherently vague about the size and range of integers. That 
would be OK, but implementing arbitrary precision integers in decimal 
notation is grossly inefficient. Representing them in hexadecimal (or 
any other binary-power base) would allow any program to operate on 
arbitrary sized integers without any concern for word size. (Because you 
can simply operate on them a nybble or byte at a time.)
> Anyway, regarding your specific "gripes" ...
>
>   
>> 1. Needs an explicit library_init/library_destroy and a
>> library_handle. (No static/global state, everything referenced by the
>> library_handle.)
>>     
>
> I not sure I really understand this but when it comes to API
> initialization my personal feeling is that a well designed API
> always has a top level context object. An example of a bad API is
> malloc/realloc/calloc/free. An example of a good API is krb5_*. The
> context object should be used to hold anything that might need to be
> initialized by the library although I think it is ok to have static data
> for certain things (e.g. error tables, character conversion contexts,
> etc).
>
> So you might have something like the following (assuming a namespace
> of ldap2):
>
>   struct ldap2_context *ctx;
>   int ret;
>  
>   ret = ldap2_init_context(&ctx);
>   if (ret < 0) {
>       // error
>       return ret;
>   }
>   // use ctx ...
>   

I think we're in agreement here - currently there is no function for 
initializing the LDAP library context. The "LDAP *" handle is just a 
handle for a single session, and it relies on global state that is 
implicitly initialized and is never deinit'd. There isn't even any 
symmetry regarding the session handles - ldap_init() creates a handle 
but doesn't do anything else; the only way to free a handle is using 
ldap_unbind() which also happens to try to send an Unbind request to the 
remote side. Completely braindead.
>   
>>      1. the API is too malloc-happy. result parsing etc. should be done
>>      in-place as much as possible.
>>     
>
> Given the highly variable nature of LDAP responses, it's not clear to me
> how you could get around this.

We actually already implemented this to a large extent in OpenLDAP 2.1. 
slapd does zero memory copying when parsing received requests, and only 
a bare minimum of mallocs for list structures. This involved adding a 
bunch of new features to liblber's ber_scanf(). These changes were 
documented but we never really talked about them or tried to get 
application writers to adopt them.

>  However I do find the memory allocation
> API very annoying. I would hope that could be normalized a little.
>
>   ldap_memfree
>   ber_free
>   ldap_value_free
>   ldap_value_free_len
>   ...
>
>   
A lot of those are still needed, but a lot of the redundant ones have 
already been deprecated.
>> 5. "proper" error handling
>>
>>      1. Any routine which can "fail" (e.g., most) should return a clear
>>      indication of success or the nature of the failure. ("fail" here
>>      refers to API failure, not a protocol failure.)
>>
>>      2. Protocol errors are not to be treated as API errors.
>>     
>
> This may not be directly related, but with respect to API design, I
> prefer to see every function return an integer that is less than 0 to
> indicate an error. For example, instead of ldap_get_dn returning char *
> it should return int and have a char ** out parameter or char *buffer
> in which the result could be returned.
>   

That is exactly what we're talking about here.
>   
>>      3. no ld_errno, no ld->ld_errno, etc.. (This relates to Howard's #3.)
>>     
>
> I actually don't think the ldap_get_option method is all that bad. At
> least calling a function to get the error result is perfectly reasonable.
>   

No. If the APIs always return an error code, as stated above, then this 
is unnecessary. And if the session handle has multiple outstanding 
requests, then ldap_get_option() is ambiguous - you have no idea which 
request corresponds to the returned error. The use of ldap_get_option 
for this purpose is totally bogus.
>   
>> 10. ldap_* be a low-level LDAP API (one-to-one mapping of API to
>> protocol). Higher level "directory" API provided above to handle referral
>> chasing and such.
>>     
>
> As I somewhat eluded to before, I pretty much just did this. I created a
> "low-level" api that *just* barely abstracted the ldap_* API so that I
> could use alternative stores (e.g. an mmaped file). Then I have a higher
> level API that is more user friendly.
>   

Understood.
>   
>> What namespaces will we use?
>>     
>
> Please don't use ldapx_*.
>   

I think that comment should go in the wiki, and the rest of this 
discussion should move to the ldapext mailing list. My posting here was 
just meant to get hold of the right people.

Thanks for the response, lots of good stuff there.
> Mike
>
>   


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