Re: (ITS#5340) REP_ENTRY_MODIFIABLE bug in dynlist

[h.b.furuseth@usit.uio.no]

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

-- 
Hallvard