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

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



masarati@aero.polimi.it wrote:
>>>> 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
>   
For debugging purposes, or something like that?
> 2) I note you put
>
>         LDAPMod **lr_mods; /* list of mods for LDAP_REQ_MODIFY,
> LDAP_REQ_ADD */
>         struct berval lr_newrdn; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
> LDAP_REQ_RENAME */
>         struct berval lr_newsuperior; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
> LDAP_REQ_RENAME */
>         int lr_deleteoldrdn; /* LDAP_REQ_MODDN, LDAP_REQ_MODRDN,
> LDAP_REQ_RENAME */
>         /* 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.
>   
Please check out this patch - 
ftp://ftp.openldap.org/incoming/openldap-2.4.21-ldifrecord-union-20100412.patch
> p.
>
>
>
>
>