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

Re: (ITS#4627) back-ldif issues



Pierangelo Masarati writes:
>> Error handling:
>>
>> ldif_tool_entry_first() cannot return NOID.
>
> Dunno (untouched)

Howard fixed.  One remaining issue: ldif_tool_entry_next() increments
li->li_tool_current even when it returns NOID.  Should it do that?

> (...)
>> 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.
>
> Fixed; now mkstemp(3) is used, and rename(2) only in case of success.

Not there yet...  you create it in the current directory instead
of in the ldif directory.

Possibly it's best to create it in the top directory of the ldif
database.  That way, if a temp file is somehow created and not
cleaned away, it's (a) in the easiest place to see and (b) won't
prevent rmdir() if one tries to delete that database or whatever.

I don't know if there are non-Unix OSes which need special system calls
to move file between directories though.  Or for that matter, if they
allow rename() to replace an existing file, maybe unlink is needed
first.  The code looks very Unixy already, so I assume a Unix emulation
is needed to run it anyway.  I don't know how those work.

Also there were some spew_entry() coding errors, rev 1.70 fixes them.

> (...)
>> .inum should be signed, or should be read with strtoul instead of
>> strtol.
>
> strtoul(3) used
>
>> I suppose it and cmp in the function should be ber_int_t,
>> since LDAP integers are typically 32-bit.
>
> Not sure what you mean.

I don't remember.  Maybe the number can end up being sent over the
protocol, and it's possible to create a filename with a long > max
ber_int_t so the wrong number will be sent?

-- 
Hallvard