ando@sys-net.it wrote:

[about REP_ENTRY_MUSTRELEASE]

it is not clear what happens when a callback chain is interrupted by slap_null_cb() or similar, without getting to slap_send_search_entry(). This seems to indicate that callbacks should always provide a last resort means to release the resources they set; if read-only, by keeping track of what they sent; if modifiable, by freeing the contents of rs->sr_* if not NULL, setting it to NULL to prevent further cleanup.

That sounds cumbersome, I hope slapd could take care of that somehow.

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

be->be_release() does receive entries that were not created by 'be' or at least not with be->be_fetch(), see openldap-devel thread 'slapd API' in mar 2008.

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 is that REP_ENTRY_MODIFYABLE without REP_ENTRY_MUSTBEFREED implies that the callback that set the former will take care of freeing the entry; however, other callbacks may further modify it, so freeing temporary data should probably be left to the final handler.

That's not my impression. MODIFIABLE would be that other modules than the creator can modify the entry - but the creator might still be the one who will free it. MUSTBEFREED is that the entry must be entry_free()ed - the creator will not do it (or not do it unless that flag is set).

So, if I'm getting this right...

A backend must expect an entry to change or be freed if it sends the entry with REP_ENTRY_<MUSTRELEASE, MUSTBEFREED or MODIFIABLE>, or if it passes through a slap_callback.sc_response.

back-ldif does not: it uses e_name/e_nname _after_ sending with with REP_ENTRY_MODIFIABLE.

Nor overlay retcode, it calls retcode_entry_response(,,,rs->sr_entry) which makes pointers into sr_entry without checking if those flags are set. If I'm getting that code correctly. I haven't tested.

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. translucent does not, even though it is a careful one which obeys both MUSTBEFREED and MUSTRELEASE.

I'm not sure when the code must clear the entry_related flags. Some code which sets new entries seem to assume they are zero, but some code which sets sr_entry=NULL does not clear the flags.

There are sr_flags bits which are not about the entry, in particular REP_MATCHED_MUSTBEFREED and REP_MATCHED_MUSTBEFREED. Looks like these flags can get lost when but some code sets sr_flags = <something> rather sr_flags |= <something> or sr_flags ~= ~<something>.