Rich Megginson wrote:
Any comments at all? Good? Bad? Don't Care?
Sorry, got busy with other things.
I have some concerns about struct ldifrecord, relatively minor though. In new APIs I prefer to use struct bervals exclusively, no naked char *s. (Ultimately, if we ever get to rewriting the C LDAP API, I would completely eliminate char *s, as we already have in the server.) Also, unless you're actually using a BerVArray, you should use actual struct bervals, not pointers to structs. I.e., unless there's some substantial benefit to having them separately allocatable, it's better to avoid the extra malloc of the berval structure and just embed the thing in the main structure. This is all standard practice for OpenLDAP code.
In ldifutil.c your main comment refers to ldap_mod_from_ldifrec() but there is no such function in the file.
The large #if 0 block /* we should faithfully encode the LDIF... should just be deleted, we will never use that code in the future. I left it in place as a reminder to myself, but when migrating code from place to place we should drop anything obsolete instead of carrying it over forever.
Rich Megginson wrote:
This patch provides low level LDIF support in libldif and high level support in libldap.
http://rmeggins.fedorapeople.org/newldif.patch
http://rmeggins.fedorapeople.org/ldifutil.c
The libldap support is in the form of
LDAP_F( int ) ldap_parse_ldif_record LDAP_P(( char *rbuf, int linenum, LDIFRecord *lr, const char *errstr, unsigned int flags, void *ctx ));
Where rbuf and linenum come from ldif_read_record; lr is new (see below); errstr is the string prefix to print for error/debug messages (usually the program name e.g. "ldapmodify"), flags are one of these: #define LDIF_DEFAULT_ADD 0x01 /* if changetype missing, assume LDAP_ADD */ #define LDIF_ENTRIES_ONLY 0x02 /* ignore changetypes other than add */ #define LDIF_NO_CONTROLS 0x04 /* ignore control specifications */
and ctx is the memory context (for ber_*alloc_x)
LDIFRecord returns all of the data from the LDIF record, regardless of what type of operation it is. The lr_op field is used to determine the type of operation, and the other fields return the operation specific data. There is also a section of several data fields used by the internal implementation (lifted almost verbatim from ldapmodify.c). This is cleaned up properly by ldap_ldif_record_done()
If the LDIF record in rbuf contains no actual record (e.g. the version: field, comments only, etc.) then the return value of ldap_parse_ldif_record() will be 0, but the lr_op field will also be 0.