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