Hallvard B Furuseth wrote:
Pierangelo Masarati writes:
Hallvard B Furuseth wrote:
But I don't see how the be_release() code can work now. It sounds like
be->be_release() functions must check (how?) that the entry was created
by 'be', and otherwise pass it on to the next overlay/backend or
otherwise to entry_free(). Might involve mucking with op->o_bd and
sr_entry->e_private, I suppose. Except maybe I'm missing some existing
magic since slapd doesn't regularly crash...
Yes, but that's trivial: e_private must be NULL for temporary entries,
and copying the entry loses it (no one is supposed to muck with it
expect the entry's creator).
OK...
And the appropriate o_bd of a
(non-modifiable) entry can be easily computed from the entry's DN.
Hm? Can only the "outermost" backend/overlay create non-modifiable
entries?
I think any overlay can replace a non-modifiable rs->sr_entry value, as
long as it restores it on the way out (unless mustbe<freed/released>).
That's why I'm not so happy with MUSTRELEASE.
Similarly, the existence of REP_ENTRY_MUSTBEFREED is not totally clear:
in principle as soon as REP_ENTRY_MODIFYABLE is set, it should imply
REP_ENTRY_MUSTBEFREED; the only difference in the semantics of the two
That's not my impression. (...)
Some functions create entries on the stack an send them MODIFIABLE - but
obviously without MUSTBEFREED. They use entry_clean() instead.
Others set MODIFIABLE but not MUSTBEFREED on non-stack entries, and call
entry_free after sending it. Or forget to free it - back-perl/search.c
if LDAP_SIZELIMIT_EXCEEDED. (Not patching yet - don't have time to test
it at the moment.)
OK, makes sense. But perhaps creating entries on the stack could be
deprecated now that entries are pooled and reused by calling
entry_alloc() entry_free(). This is just food for thoughts, we could as
well leave all those flags 'round, if we can streamline their handling.
(...) A copy might be created by an overlay after receiving a
read-only entry, but the same overlay might not actually perform the
copy if it receives the entry from another overlay that already copied
it, or from a proxy backend. However, after the entry is copied the
overlay will have no means to determined who actually created the copy.
I don't think it matters who created a MUSTBEFREED copy:
right.
This might be an issue depending on the order cleanup handlers are
called (didn't check what order they're called). My point is that
temporary entries need to be freed at some point; who frees them should
not be relevant...
Right, someone must call entry_free() and clear rs->sr_entry (so it's
not freed again), but entry_free() is not passed a backend parameter.
OTOH a MUSTBERELEASED entry must be released by the right
backend/overlay, and it would be strange for such an entry to be
MODIFIBALE. Though I guess it could be - if the backend has no cache,
and be_release just exists to clean up some data in e_private.
Others apparent problems (also not tested, I've just browsed the code):
Overlays that obey and reset MUSTBEFREED or MUSTRELEASE, do not
necessarily clear or set MODIFIABLE when setting a new entry.
(...)
It seems to me that we should provide "smart" handlers to deal with
preparing sr_entry for modification, and to take care of cleaning things
up as appropriate. Those helpers should then be consistently used in
the code.
Yup. And add some aggressive asserts - at least #ifdef LDAP_DEVEL.
Don't know when I'll have time for it though.
Making room in my todo list...
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
---------------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Email: pierangelo.masarati@sys-net.it
---------------------------------------