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

Re: (ITS#4314) C API for paged results

I've committed something that looks like (ldap.h)

LDAP_F( int )
ldap_create_page_control_value LDAP_P((
        LDAP             *ld,
        unsigned long    pagesize,
        struct berval    *cookie,
        struct berval   *value ));

LDAP_F( int )
ldap_create_page_control LDAP_P((
        LDAP             *ld,
        unsigned long    pagesize,
        struct berval    *cookie,
        int             iscritical,
        LDAPControl      **ctrlp ));

LDAP_F( int )
ldap_parse_page_control LDAP_P((
        LDAP           *ld,
        LDAPControl    **ctrls,
        unsigned long  *count,
        struct berval  **cookie ));

LDAP_F( int )
ldap_parse_pageresponse_control LDAP_P((
        LDAP           *ld,
        LDAPControl    *ctrl,
        unsigned long  *count,
        struct berval  *cookie ));

ldap_create_page_control() uses ldap_create_page_control_value() and
should be deprecated, IMHO, as it favors an inefficient style of using
controls in C programming.

ldap_parse_page_control() uses ldap_parse_pageresponse_control(); it is
consistent with existing LDAP C API, but it should be deprecated as well
for the same reason above.  In general:

ldap_create_*_control() may lead to poor programming, but they're not
too bad by themselves; all in all there might be cases where the list of
controls must be entirely malloc()'ed.

ldap_create_*_control_value() should be used instead, whenever possible,
with LDAPControl structs on the stack.  Controls like manageDSAit,
passwordpolicy and so don't even need this type of helper because they
have empty value.

ldap_parse_*_control() functions should also be deprecated; in fact,
they imply control lookup in an array, and, in general, unnecessary
allocation to return results.  I consider unnecessary allocation, for
example, when a berval is malloc()'ed to mark the distinction between no
return value and an empty return value if the semantics of the control
do not require that distinction.

ldap_parse_*response_control() functions directly take a pointer to the
LDAPControl structure in input; to further mark the distinction with
older functions, we could just pass a pointer to the control value, as
they do not need anything else: the OID is implicit in the function
name, and the criticality __must__ be FALSE, so control response
semantics cannot depend on the rest of the LDAPControl structure.

Please check my commits and report.


On Sun, 2006-01-08 at 22:35 +0100, Hans Leidekker wrote:
> On Sunday 08 January 2006 19:16, Kurt D. Zeilenga wrote:
> > I would like to this be reworked using ppolicy API as a basis than
> Can you specify what you mean by "ppolicy API"? I presume you
> mean taking a single control instead of an array?. And assert()ing
> on faulty parameters maybe? Anything else?
> I see a function ldap_parse_passwordpolicy_control() in ppolicy.c 
> that does inline berval parsing, which contrasts with what Pierangelo
> said about splitting it up.
> > the older sort/vlv control APIs (which should also be reworked).
> > Parse routines shouldn't also find the control as this makes it
> > harder to use in programs that include their own finding.
> Hmm, anything else? ;-) So if I take into account what Pierangelo
> and you wrote I arrive at the following API:
> int
> ldap_create_page_control(
>     LDAP           *ld,
>     unsigned long  pagesize,
>     struct berval  *cookie,
>     int            iscritical,
>     LDAPControl    **ctrlp );
> int
> ldap_parse_page_control (
>     LDAP           *ld,
>     LDAPControl    *ctrls,
>     unsigned long  *count,
>     struct berval  *cookie );
> Changes relative to the previous version:
>   - change iscritical from a "const char" to an "int".
>   - accept an "LDAPControl *" instead of an "LDAPControl **"
>   - return a "struct berval *" instead of a "struct berval **". 
>  -Hans

Ing. Pierangelo Masarati
Responsabile Open Solution
OpenLDAP Core Team

SysNet s.n.c.
Via Dossi, 8 - 27100 Pavia - ITALIA
Office:   +39.02.23998309          
Mobile:   +39.333.4963172
Email:    pierangelo.masarati@sys-net.it