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: [EMAIL PROTECTED] ---------------------------------------
