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

Re: More type problems and return values in the C API



Hallvard B Furuseth wrote:
> 
> Since I'm writing about integer types: I don't remember which of these
> problems I've mentioned before: Here are some other doubtful `int'
> values, enclosed in /*...*/.
> 
> Maybe these should be ber_int_t:
> 
>   /*msgid*/  ldap_add();
>   /*msgid*/  ldap_bind();
>   /*msgid*/  ldap_compare();
>   /*msgid*/  ldap_delete();
>   /*msgid*/  ldap_kerberos_bind();
>   /*msgid*/  ldap_modify();
>   /*msgid*/  ldap_modrdn();
>   /*msgid*/  ldap_modrdn2();
>   /*msgid*/  ldap_msgid();
>   /*msgid*/  ldap_search();
>   /*msgid*/  ldap_simple_bind();
>              ldap_abandon           (,       /*msgid*/      );
>              ldap_abandon_ext       (,       /*msgid*/   ,, );
>              ldap_add_ext           (,,,,,   /*msgid*/ *    );
>              ldap_delete_ext        (,,,,    /*msgid*/ *    );
>              ldap_extended_operation(,,,,,   /*msgid*/ *    );
>              ldap_modify_ext        (,,,,,   /*msgid*/ *    );
>              ldap_rename            (,,,,,,, /*msgid*/ *    );
>              ldap_result            (,       /*msgid*/  ,,, );
>              ldap_sasl_bind         (,,,,,,  /*msgid*/ *    );
>              ldap_search_ext  (,,,,,,,,, /*sizelimit*/, /*msgid*/ * );
>              ldap_search_ext_s(,,,,,,,,, /*sizelimit*/, );
>              ldap_*_option(, LDAP_OPT_<SIZE/TIME>LIMIT, /*limit*/ * );
> 
> If we want support for more than 32767 values & so on, maybe these
> should be ber_int_t (or ber_slen_t as I suggested for ber_printf(), if
> that type is introduced):
> 
>   /*count*/  ldap_count_entries();
>   /*count*/  ldap_count_messages();
>   /*count*/  ldap_count_references();
>   /*count*/  ldap_count_values();
>   /*count*/  ldap_count_values_len();
>
> I guess `int' is enough for current and future LBER_* options?
> 
>   BerElement *ber_alloc_t( /*options*/ );


I think we have discussed this before (although maybe it was in private
email and not on the list -- I don't remember).  The last paragraph of
section 5 of the draft says "Note that this API is designed for use in
environments where the 'int' type is at least 32 bits in size." 
Therefore, I don't think it is too limiting to use int for the
parameters you mention.



> A few other points:
> 
> The draft doesn't seem to say what ldap_msgfree() returns on
> success.  (OpenLDAP returns the type of the last freed message.)

Actually, this is already covered.  This text appears in section 12
(third to last paragraph): 

   ldap_msgfree() frees the result structure pointed to by res and
   returns the type of the message it freed.  If res is NULL, nothing
   is done and the value zero is returned.


> ...
> What do ldap_count_<messages,entires,references>() return on error?
> (OpenLDAP returns -1.)

-1 is a good choice I think.


> ...
> Typo:
> 
> > All three of the ldap_parse_*_result() routines skip over messages of
> Should be          ldap_parse*_result()
> in order to match "ldap_parse_result()".
> 
> Typo:  Missing '.' at the end of this sentence in section 7:
> 
> > The LDAP structure is an opaque data type that represents an LDAP ses-
> > -sion

I'll fix both of these too.  I think I'll eliminate the
ldap_parse*_result*() shorthand entirely and just list the three
functions as that will make the text clearer.

Thanks again!

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