Re: extensions/opts in ldap-c-api-01

[mcs@netscape.com (Mark C Smith)]

"Kurt D. Zeilenga" wrote:
> 
> While experimenting with a few api extensions and associated options,
> I found a few areas I believe the draft could use some improvement.
> 
> The draft provides a mechanisms for determining which extensions are
> available at both compile time and run time but only provides the
> feature "level" at compile time. I believe applications will need
> to be able to determine "level" of each feature at compile time as
> well.  This is especially important in dynamically link environments
> when multiple "levels" of the same feature are be widely available.
> ...

I assume you meant to say '...determine "level" of each feature at RUN
TIME as well'?  I agree; we might as well make as much information as
possible available to application programmers (within reason of course
;-)


> ...
> I recommend that structure returned by LDAP_OPT_API_INFO be extended
> to provide feature levels that correspond to the value of the associated
> LDAP_API_FEATURE_X compile time flag.
> 
> One mechanism to do this would be to modify LDAP_OPT_API_INFO to
> return:
> 
>   typedef struct ldap_apifeature_info {
>     char *ldapaif_name;         /* matches LDAP_API_FEATURE_... less the prefix */
>     int ldapaif_version;        /* matches the value LDAP_API_FEATURE_... */
>   } LDAPAPIFeatureInfo;
> 
>   typedef struct ldap_apiinfo {
>     int ldapai_info_version; /* version of LDAPAPIInfo (2) */
>     int ldapai_api_version; /* version of API supported */
>     int ldapai_protocol_version; /* hightest LDAP version supported */
>     LDAPAPIFeatureInfo *ldapai_features; /* names and levels of supported features */
>     char *ldapai_vendor_name; /* name of supplier */
>     int ldapai_vendor_version; /* name of vendor versions */
>   } LDAPAPIInfo;
> 
> Applications can use this information to verify at runtime that the
> appropriate "level" is provided.  The drawback is that ldap_value_free()
> can be used for deallocation of this field.  An alternative would be
> to add a new field (int *ldapai_extensions_levels) to the existing
> structure (instead of a replacing ldapai_extensions).  This is ugly,
> but allows for much easier deallocation.

My vote would be to add a structure similar to what you suggest and just
provide a ldap_apifeature_info_free() function to take care of the
deallocation.


> ...
> Another alternative is to fetch feature information with ldap_get_option().
> Something like:
>   LDAPAPIFeatureInfo = f;
>   f.ldapaif_name = "FUBAR";
>   ldap_get_option(NULL, LDAP_OPT_API_FEATURE_INFO, &feature)
>   if( f.ldapaif_version < 0 ) { not implemented } else ...;
> 
> I think the last mechanism.  This could be implemented as an extension
> (feature) to the API.

I think I like this better than your first proposal since my guess is
that most extensions will be simple and the level will rarely need to be
checked by applications.  I could be wrong about that though.  I do
think we should keep the ldapai_extensions field in the LDAPAPIInfo
struct returned by the ldap_get_option( ..., LDAP_OPT_API_INFO, ... )
call so the names of all the extensions can be retrieved.


> ...
> On the subject of LDAP_OPTs, we should do something to keep vendor
> specific LDAP_OPT_ names/values from clashing with future *standard*
> options names/values.  I recommend all vendor/extended opts be named:
>   LDAP_OPT_X_VENDOR_...
> 
> and have assigned values be between 2^14 and 2^15-1 inclusively.  Values
> below 2^14 would be reserved by the API for itself and standard API
> extensions. 2^15 and above would not be used.

Sounds like a good suggestion, although some widely deployed vendor
implementations such as Netscape's already use lower numbers for their
own extensions.  We can transition away though.


> ...
> On a side note, shouldn't we provide a compile time mechanism for checking
> the vendor name and version?
> 
>   if ( strcmp(LDAP_VENDOR_NAME, ai->ldapai_vendor_name) ) {
>     /* vendor mismatch, I hope this vendor's implementation is okay */
>   } else {
>     if ( ai->ldapai_vendor_version <  LDAP_VENDOR_VERSION ) {
>       /* using an old vendor version... could be buggy */
>     } else {
>       /* vendor version is same or newer than we compiled with */
>     }
>   }
> 
> I believe this would very useful to application developers and is trivial
> to implement.

Unless someone objects, I will add these two macros (LDAP_VENDOR_NAME
and LDAP_VENDOR_VERSION) to the next revision of the draft.

Thank you for all of your helpful suggestions!

-- 
Mark Smith
Directory Architect / Netscape Communications Corp.
My words are my own, not my employer's.  Got LDAP?