Subject: ber_printf's return type in the C API

From: Hallvard B Furuseth <[email-masked]>
Date: Wed, 4 Aug 1999 02:36:54 +0200

Yet Another API Nitpick...

As far as I can tell, ber_printf() usually returns the number of octets
written from the last parameter, or -1 on error.  Implementations that
want to do that reliably will need the return type of ber_printf to be
"signed ber_len_t" (OpenLDAP calls it ber_slen_t) instead of int,
otherwise they can't output objects with more than MAX_INT octets.
(They still can't print objects whose size is in the range
<max(ber_slen_t),max(ber_len_t)], though.)

Personally I don't care, but I imagine it could matter to whoever it was
who insisted that we can't give ber_printf and ber_scanf the same error
return (and return type), because it breaks backwards compatibility.

BTW, ber_printf(ber,"") returns 0 on the implementations I have seen.
This violates the clain "ber_printf() returns ... a positive number if
successful", it should be "nonnegative".  The same goes for the
"positive" error codes in section 9 paragraph 1.

-- 
Hallvard

---

From: Hallvard B Furuseth <[email-masked]>
Date: Thu, 5 Aug 1999 02:56:43 +0200

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*/ );

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.)

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

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

-- 
Hallvard

---

From: Mark Smith <[email-masked]>
Date: Thu, 05 Aug 1999 11:21:50 -0400

Unless I missed it, the current draft only requires that "ber_printf()
returns -1 if there is an error during encoding and a positive number if
successful."  Therefore, it is okay to leave the return type as int.  Is
it important for the caller to know how many octets were output?  I
don't think so myself.  If that functionality is required though, we
will need to do something about the ber_printf() return value.

I do agree that the text should be changed to say "... a non-negative
number if successful" and also in section 9 paragraph 1.  Good catch!

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

---

From: Mark Smith <[email-masked]>
Date: Thu, 05 Aug 1999 11:45:08 -0400

>   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.

> 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.

> ...

-1 is a good choice I think.

> ...

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!

---

From: Hallvard B Furuseth <[email-masked]>
Date: Fri, 6 Aug 1999 00:23:24 +0200

Mark Smith writes:

> 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.

OK.

> (third to last paragraph):

Duh.  Sorry about that.

>    is done and the value zero is returned.

However, it doesn't say which message's type is returned if it frees a
chain of messages (2nd paragraph in section 12).  Or have all messages
in a chain the same type?

-- 
Hallvard

---

From: Hallvard B Furuseth <[email-masked]>
Date: Fri, 6 Aug 1999 00:32:01 +0200

Mark Smith writes:
> it important for the caller to know how many octets were output?

No, like I said: personally I don't care.

I was wondering if someone _else_ care, namely the people objected to
giving ber_printf and ber_scanf the same way to report success/error
because their used the returned values for something internally in their
software.  (In fact, that may have been you Netscape people - I don't
quite remember.)

-- 
Hallvard

---

From: Mark Smith <[email-masked]>
Date: Fri, 06 Aug 1999 14:12:41 -0400

> in a chain the same type?

No, they do not necessarily have the same type (a chain returned by a
search operation might contain entries, references, and a result, for
example).  I will change the text to read:

   ldap_msgfree() frees each message in the result chain
   pointed to by res and returns the type of the last message
   in the chain.  If res is NULL, nothing is done and the
   value zero is returned.