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 ---------------------------------------