ldap_get/set_option in ldapext-ldap-c-api-01

["Kurt D. Zeilenga" <Kurt@OpenLDAP.Org>]

The API draft features for accessing global and session
parameters via the accessor functions ldap_get_option()
and ldap_set_option().  I do have a few comments and
suggestions based on our early implementation experiences.

Section 9.2, p11-14


ERROR HANDLING
The specification does not provide a mechanism for the
accessor functions to return what type of error occurred.
I recommend that accessor functions return error codes.


OPTION VALUES
Option values are currently "int"s.  Recommend using
an unsigned type, preferably "unsigned long".

Option names for extensions should be described.  Recommend
using a convention similar to LDAP_API_FEATURE_x, ie:
LDAP_OPT_x.  All options for extensions should be associated
with a previously defined feature and similarly named:  Ie:

LDAP_API_FEATURE_X_OPENLDAP_TURBO
LDAP_OPT_X_OPENLDAP_TURBO_INFO
LDAP_OPT_X_OPENLDAP_TURBO_BOOST

Extended options numbers should be above some arbitrary value.
Say, the bottom 1/4 of the range reserved for API defined
options, the next 1/4 reserved for draft/experimental, the
remaining reserved for vendor extensions.

WRITE-ONLY
The specification should state that some options are write-only.


DISPOSING OF MEMORY (p 14)
Editorial correction:
The specification states that outvalues of type "char *" and
LDAPControl * need not be disposed of by the caller.   I believe
the intent for this was to be "char **" and "LDAPControl ***"
as these are the outvalue types for LDAP_OPT_HOST_NAME,
LDAP_OPT_ERROR_STRING, LDAP_OPT_{CLIENT,SERVER}_CONTROLS.

I believe the API should ALWAYS require the caller to dispose of
memory associated with ldap_get/set_option calls.  Allowing
exceptions is a slippery slope...
 
Simple example.  caller asks for global host name.  Implementation
provides a copy of the global host name.  The implementation can
never safely dispose of the associated memory!


GLOBAL vs SESSION OPTION CLARIFICATION

Which options are session only?
Can this interface be used to specify global-only options?


SPECIFIC OPTIONS

LDAP_OPT_TIMELIMIT
I'd like a time limit of a half a second?  Recommend
using timeval instead of int to represent the time limit.

LDAP_OPT_ERROR_NUMBER
I suggest that if the API allows writes, it only allow clearing
of the ERROR_NUMBER, that is, setting it to LDAP_SUCCESS.  

LDAP_OPT_ERROR_STRING
I suggest that if the API allows writes, it only allow clearing
of the ERROR_STRING, that is, setting it to NULL.  

Kurt