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

Re: (ITS#6194) Patch - Enhancement - provide LDIF support as libldif

>>> Sorry about that.  You can find it here:
>>> http://rmeggins.fedorapeople.org/ldifutil.c
>> Gotit, thanks.  I have one first comment: the public API does not
>> generally expose the memory context.  I'm renaming
>> ldap_parse_ldif_record() as ldap_parse_ldif_record_x(), and eliminating
>> the void *ctx arg from ldap_parse_ldif_record(), if you don't mind.
> tested and applied to HEAD; please test.  Thanks, p.

More comments:

1) perhaps it may be worth providing a function that converts a LDIFRecord
structure into a string, and one that sends it to a stream

2) I note you put

        LDAPMod **lr_mods; /* list of mods for LDAP_REQ_MODIFY,
        struct berval lr_newrdn; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
        struct berval lr_newsuperior; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
        int lr_deleteoldrdn; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
        /* the following are for future support */
        struct berval lr_extop_oid; /* LDAP_REQ_EXTENDED */
        struct berval lr_extop_data; /* LDAP_REQ_EXTENDED */
        struct berval lr_cmp_attr; /* LDAP_REQ_COMPARE */
        struct berval lr_cmp_bvalue; /* LDAP_REQ_COMPARE */

in the structure; the last four are not used right now.  I think it would
make sense to group struct members by op in substructures, and then put
them in a union, to stress the fact that they're mutually exclusive.  Much
like Howard did for the corresponding substructures in the Operation
struct in slapd.