[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: c-api-03 minor nits



"Kurt D. Zeilenga" wrote:
> 
> This posting covers a few minor nits.  I'll post major nits
> separately.
> 
> 1) memory handling semantics (clarification)
> 
> The ber_bvfree() description includes: "Applications should not
> use ber_bvfree with bervals which the aplication has allocated."
> This applies, I believe to every *free* routine in the API.
> However, many API calls do not include such in their descriptions.
> 
> I believe it would be wise to add a section describing, in
> general, the API memory allocation semantics and than detailing
> only exceptions in individual call descriptions.

Agreed -- I'll try to gather up the common memory handling semantics and
put them in one place.  The following should be generally true:

a) Applications should not call any of the C LDAP API's "free" functions
unless the memory was allocated by the API implementation, i.e., not if
they allocated it themselves.

b) If the pointer value passed to a C LDAP API "free" function is NULL,
graceful failure (or ignoring of the NULL pointer) occurs.  This is also
not mentioned everywhere it should be in the document.

The "free" functions are:
   ber_bvecfree()
   ber_bvfree()
   ber_free()
   ldap_control_free()
   ldap_controls_free()
   ldap_memfree()
   ldap_msgfree()
   ldap_value_free()
   ldap_value_free_len()


> ...
> 2) struct berval
> 
> Other structure defined by the API have typedefs associated with
> them.  I suggest adding:
>         typedef struct berval { ... } BerVal;  /* or BerValue */
> 
> Purely for symmetry...

Okay by me.


> ...
> 3) struct ldapmod
> 
> The union (mod_vals) should not be anonymous.  Suggest:
>         union mod_vals_u {
>                 ...
>         } mod_vals

Can you educate me as to why it matters that it is anonymous?  This
isn't a big change, but I don't know what problem we are fixing with it.


> ...
> 4) PLDAP namespace
> 
> PLDAP symbols are not used nor are they typedef'ed for all
> LDAP structures (nor is there any PBer typedefs).   As these
> symbols are not used by the specification, I suggest that
> all PLDAP symbols be removed from the specification and the
> namespace consumed by the specification.  (This still allows
> implementations to provide PLDAP symbols if they choose to,
> this change removes PLDAP symbols from the scope of this
> specification).

Okay by me.  The only PLDAP definition is for PLDAPControl (a pointer to
an LDAPControl structure).  Any objections?


> ...
> 5) Tag constants
> 
> The document includes numerous BER tags (in macros and in
> examples).  These should all be declared with a 'U' suffix
> (most are declared bare, some are declared L).
> 
> Ie:
> #define LDAP_CHASE_SUBORDINATE_REFERRALS 0x00000020U
> 
> (note to editor, we sure to catch 0xn tags used in examples).

I'll do my best to find and fix all of these.


> ...
> 6) struct timeval
> 
> Not all systems timeval fields are of type 'long'.
> Implementations must be allowed to declare struct
> timeval as it is found in primary header files of
> the current C translator.

I didn't realize that was the case.  Is timeval defined in a POSIX spec.
(it must be)?  It would be nice to have a reference.

Thanks for these helpful comments.

-- 
Mark Smith
Netscape Communications Corp. / Directory Server Engineering
"Got LDAP?"