(ITS#4627) back-ldif issues

[h.b.furuseth@usit.uio.no]

Full_Name: Hallvard B Furuseth
Version: HEAD
OS: 
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (129.240.186.42)
Submitted by: hallvard


Here are a number of back-ldif quirks and bugs.

Error handling:

ldif_tool_entry_first() cannot return NOID.
ldif_back_<add,modify,delete,modrdn> always return success (0).
Maybe some of this is a requirement of back-config?  If so I
suggest that back-config sets a ldif_info.li_optimism flag and
that back-ldif otherwise treats errors as errors.

If rmdir() in ldif_back_delete() fails with errno != ENOENT,
rs->sr_err is set to LDAP_NOT_ALLOWED_ON_NONLEAF - but is then
immediately overwritten with LDAP_UNWILLING_TO_PERFORM.

ldif_tool_entry_put() sets 'res' to various LDAP result codes, but
only uses it as a boolean.  Are the codes supposed to be logged?

Files:

back-ldif overwrites files instead writing to a temp file and
renaming when complete.  The current way corrupts entries if a
write fails halfway through, e.g. due to a full disk. Also the
file can be left truncated if internal slapd operations fail.
If the OS supports it, back-ldif should make a new file in the
same directory and rename to the old filename when complete.

Locks:

The global entry2str_mutex is locked around spew_entry() instead
of inside it, so it spans more file operations than necessary.
Should be held more briefly, I think.  Though it does make kind of
sense now: It is locked before the file is truncated so that the
file won't be in truncated state for long while slapd waits for
the mutex.  But that's not quite how I'd go about solving that
problem; see above.  (I have not checked if the mutex doubles as a
mutex to synchronize internal back-ldif operations.  So I don't
know if lock/unlock can just be moved inside spew_entry().)

struct bvlist quirks:

.num holds an unnecessarily duplicated string.  It only needs to a
bechar, holding the character being overwritten with a '\0'.  (And
replace the AC_MEMCPY in r_enum_tree() which restores it.)

For that matter, .num is not needed at all if the '\0' can replace
the IX_FSL (usually '{') instead of the character following it.
(And decrement .off with 1.)  Then r_enum_tree() can just put back
an IS_FSL.  It would affect the sort order of values prefixed with
'{num}' compared to unprefixed values of the same type.

.inum should be signed, or should be read with strtoul instead of
strtol.  I suppose it and cmp in the function should be ber_int_t,
since LDAP integers are typically 32-bit.

Also, for reading .inum and syntax checking: Maybe you should use
strtol() without first doing strchr(,IX_FSR), and then check that
the character after the integer is IX_FSR.  Slower though.

Other quirks:

ldif.c uses "struct ldif_info *ni" = be->be_private.  Maybe it
used back-null as a template, where "ni" stands for null_info?

Return values from get_entry() and spew_entry() are pointlessly
cast to the same type as the functions already return.