[Date Prev][Date Next]
Re: Providing LDIF support at the LDAP level
- To: Howard Chu <firstname.lastname@example.org>
- Subject: Re: Providing LDIF support at the LDAP level
- From: Rich Megginson <email@example.com>
- Date: Fri, 29 Jan 2010 08:49:28 -0700
- Cc: OpenLDAP Devel <firstname.lastname@example.org>
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:reply-to :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=CmPICr6jlQHwJqWmRzBSGkAfJn2w/dFfyoWDKGcBz5s=; b=k7NfGNtFehhUNEK7dNlRL3yM9kZx03QAhZfPLOQWrKDTWdVqcGq+yKFDsmwzu/f3mp 3e9vh+EmlBWIylco/2HiUmyQwAfR9Kosig6wQNCfBfZod/qwMlweYGnYbhOkl5qPWGEf QlsxgEZp5p8AM0i6oFGIDCZPhZS7SGXrcRb/E=
- Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=wtNC0z46RUF18funDtXnT+gDGZTfEOBcwk6L1Y38KY/KNvLW/GqNDT0PSr9s3BK4yV bDLTekIBN6zKAF6L+KfdGV6qAqW3HmDGwIditNOReBZTdVtlRW4jjJgrL7uLLr4/1FPX hrb159fU2UuN6v2r/KT21/HEFi5KBcld9BIT8=
- In-reply-to: <4B5F7525.email@example.com>
- References: <4B577983.firstname.lastname@example.org> <4B5F5DB2.email@example.com> <4B5F7525.firstname.lastname@example.org>
- User-agent: Thunderbird 184.108.40.206 (X11/20090609)
Howard Chu wrote:
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.)
What about the const char *errstr parameter to
ldap_parse_ldif_record()? That's only used for logging purposes.
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.
The libldap support is in the form of
LDAP_F( int )
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
#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.