Last call comments with LDAP C API #4

[Mark Wahl <M.Wahl@INNOSOFT.COM>]

Mark, here are some comments I received, primarily from our engineers, 
on the C API.  If they look reasonable to you, I would consider that most of 
them could be treated as last call clarification requests.   Let me know if
you have any issues with them.


1) In 11.3.1 it says "The ldctl_value.bv_val field MUST point to a 4-octet 
integer flags value."

This is not 'integer' in the sense of a C int (since a C int may not be 4 
bytes), nor is it an ASN.1 INTEGER which is variable length.  This sentence
therefore needs to specify the encoding of an integer value in 4 octets.  In 
particular, the endian of this value needs to be specified.  Host byte order 
or network byte order?  

I (--mfw--) assume host, so "The ldctl_value.bv_val field MUST point to a 4 
byte area containing the flags integer value in host byte order."

2) In 11.6 on the timeout arg:

>             For the ldap_search_ext() and ldap_search_ext_s() func-
>             tions, the timeout parameter specifies both the local
>             search timeout value and the operation time limit that is
>             sent to the server within the search request.  Passing a
>             NULL value for timeout causes the global default timeout
>             stored in the LDAP session handle (set by using
>             ldap_set_option() with the LDAP_OPT_TIMELIMIT parameter) to
>             be sent to the server with the request but an infinite
>             local search timeout to be used. 

This implies either 
 - ldap_search_ext() with either a NULL or a non-NULL timeout MUST block 
    waiting for results, or
 - The timeout from an ldap_search_ext() establishes a maximum timeout for
    subsequent calls to ldap_result.  

This is not the same as the other operations, where the non _s form does not 
block. 

The ldap_search_ext() call should not block, and we should not have to propgate
the timeout and check subsequent ldap_result calls' timeouts to reduce them 
as necessary.

I (--mfw--) propose for consistentcy with the other ops we replace this 
paragraph with two paragraph:

             For the ldap_search_ext() function, the timeout parameter
             specifies the operation time limit that is sent to the server 
             within the search request.  Passing a NULL value for timeout 
             causes the global default timeout stored in the LDAP session 
             handle (set by using ldap_set_option() with the 
             LDAP_OPT_TIMELIMIT parameter) to be sent to the server with 
             the request and the call returns immediately without waiting
             for results.  If a zero timeout (where tv_sec and tv_usec are 
             both zero) is passed in, API implementations SHOULD return 
             LDAP_PARAM_ERROR.  If a zero value for tv_sec is used but 
             tv_usec is non-zero, an operation time limit of 1 SHOULD be 
             passed to the LDAP server as the operation time limit.  For 
             other values of tv_sec, the tv_sec value itself SHOULD be 
             passed to the LDAP server.

             For the ldap_search_ext_s() function, the timeout parameter 
             specifies both the local search timeout value and the operation 
             time limit that is sent to the server within the search request. 
             Passing a NULL value for timeout causes the global default 
             timeout stored in the LDAP session handle (set by using
             ldap_set_option() with the LDAP_OPT_TIMELIMIT parameter) to
             be sent to the server with the request but an infinite
             local search timeout to be used: the call will not return 
             until the search results are returned.  If a zero timeout (where
             tv_sec and tv_usec are both zero) is passed in, API imple-
             mentations SHOULD return LDAP_PARAM_ERROR.  If a zero value
             for tv_sec is used but tv_usec is non-zero, an operation
             time limit of 1 SHOULD be passed to the LDAP server as the
             operation time limit.  For other values of tv_sec, the
             tv_sec value itself SHOULD be passed to the LDAP server.

And later in this section

>  After the local search timeout expires, the API implementation will send an 
> abandon operation to abort the search operation.

First of all, does this apply to ldap_search_st only, or to ldap_search_ext 
and ldap_search_ext_s as well?  Also I assume that if the search result done 
is received the operation need not be abandoned.

Either way, this needs to be listed as a known incompatiblity with 1823, in 
which ldap_search_st does not cause an operation to be abandoned.  

In particular I (--mfw--) think this should NOT apply to ldap_search_ext, for 
consistency with the above.   I (--mfw--) propose replacing this paragraph 
with two:

 The ldap_search_ext() and ldap_search_ext_s() functions support LDAPv3
 server controls, client controls, and allow varying size and time limits
 to be easily specified for each search operation.  The ldap_search_st()
 function is identical to ldap_search_s() except that it takes an addi-
 tional parameter specifying a local timeout for the search.  

 The local search timeout argument of ldap_search_ext_s() and ldap_search_st() 
 is used to limit the amount of time the API implementation will wait for a 
 search to complete.  After the local search timeout expires, the API 
 implementation SHOULD send an abandon operation to abort the search 
 operation.

3) In 11.12 a suggestion received for the attrs parameter: might be clearer if 
stated as "Only the LDAP_MOD_BVALUES bit of the mod_op field is used, which 
causes the operation to use the mod_bvals case of the mod_vals union 
instead of the mod_strvals.  The other bits of the mod_op field are ignored."

4) In 13 a clarification request for the timeout field: I think they mean
"A timeout value of zero seconds and zero microseconds specifies a polling 
behavior."

5) In 14 a suggestion for the ldap_err2string result: "It returns a pointer to 
static data which MUST NOT be modified."

6) In 17.2 a change for the description of ber_free: "Except when releasing a 
BerElement that has been obtained from a ldap_first_attribute, the second 
argument fbuf SHOULD always be set to 1 to ensure that the internal buffer 
used by the BER functions is freed as well as the BerElement container itself."

7) In 17.3 clarify whether you can continue to add to a BerElement after it is
flattened.  

I (--mfw--) suggest explicitly say application cannot.

8) In 17.5: if the BER element len is > sizeof(int), the behavior is specified 
for enumerated and integer but not for boolean.  

9) In 17.5 last paragraph might be helpful to say "In particular the opaque 
pointer does not need to be freed by the application."

Mark Wahl, Directory Product Architect
Innosoft International, Inc.