[Copying some discussion from IRC, and adding some more]

I'm a bit nervous about doing more than minimal fixes to this until we do something major, because it's so broken. Fix one bit and maybe something will pop up another place - i.e. a regression.

Also following existing practice can mean fix one bug into a different bug, which Ando recently did for ITS#6423. (dynlist resets MUSTRELEASE but doesn't obey it, rwm works around a similar bug in rwm or dynlist.) Though he got other fixes right. Anyway:

To repeat, the main problems with the current code:

* Code does not always clean up properly, leaving e.g. flags behind.

* Code which does clean up sometimes does it halfway, e.g. it resets MUSTBEFREED/MUSTRELEASE but not MODIFIABLE. So, no wonder there are unwanted MODIFIABLE flags hanging about (ITS#6423). Or it dup()s an entry but does not release the old one.

* op_*() generally expects SlapReply to be preinitialized. That worked before overlays, but now modules (backends, overlays) inherit values that the previous module did not clean up during an operation.

Howard suggested that we should restore that - code which puts something into a SlapReply should ensure it's taken out again.

I've started on that in the enclosed patch, but on second thought I think that's not the right fix because it's the most fragile solution: Practically every module which depends on it can break if some other module gets it wrong.

OTOH, modules can't just reset SlapReply since some of it is input values to op_*():

- Controls can be set in backend_check_restrictions() before the call to op_*().

- dynlist.c sets r.sr_attr_flags before calling o.o_bd->be_search(), I haven't looked at what effect that has.

I'm thinking modules should instead initialize SlapReply.sr_u and maybe some sr_flags themselves instead of depending on preinit, but I don't know when I'll get back to doing something about that. Xmas coming...

So anyway, here is a quick patch which actually tries to be rather conservative, sans a bunch of RS_ASSERT() lines which can be safely deleted in real code. It only addresses sr_entry and sporadically attrs, not sr_refs/ctrls. It passes make test for me. Please review.

Where updates to flags are broken and needs fixing, I change to use the new function rs_ensure_entry_modifiable() etc, but I leave correct updates alone - as well as some updates where I don't know which overlay should be passed to these function.

Description of changes:

All over the place: Clear MUSTBEFREED/MUSTRELEASE by clearing all of REP_ENTRY_MASK, not just the particular flag. Reset attrs, entry, flags a number of places. (Not very consistently, sorry.)

Lots of RS_ASSERT()s for debugging. Only defined when LDAP_DEVEL.

proto-slap.h, result.c: Move RS_ASSERT(), add rs_flush_entry().

result.c: MODIFIABLE is independent of MUSTBEFREED - the modifiable entry can e.g. be on the stack.

result.c, extended.c: When changing sr->sr_type to extended/sasl, flush sr_un (usually the entry) first, since another member of the union will be used.

For this reason, updates to sr_type are "suspicious" code to be avoided, so some code elsewhere is changed to initialize it to REP_RESULT rather than setting it to that after initialization.

bconfig.c, servers/slapd/syncrepl.c: Avoid reusing SlapReply a number of places where that's easy.

slap.h: Cast the REP_... flags to slap_mask_t, so ~REP_... will work right.

back-bdb/search.c: Delay setting rs->sr_entry until we know it'll be used. (IIRC made a difference around a paged results response.)

back-shell/result.c, back-sock/result.c: Set MUSTBEFREED before sending, obey it afterwards.

overlays/constraint.c, overlays/pcache.c: Drop nulling of fields that were just nulled by initialization.

overlays/dynlist.c: When "stealing" an entry from SlapReply, grab and reset the flags that go with it, so it can be properly freed/released. Restore the flags and entry later, then obey the flags (release/free). Do this unconditionally, it happens only once per SlapReply anyway.

overlays/pcache.c: Obey and reset entry flags properly.

Remove unused SlapReply parameters to some functions - did this for readibility while trying to figure out what was going on.

overlays/sssvlv.c: Since setting MUSTRELEASE before sending, obey it afterwards.

overlays/translucent.c: Reset flags properly too when releasing/freeing entry. Streamline translucent_search() loop to clarify ownership of entry.

overlays/rwm.c, servers/slapd/slapi/slapi_pblock.c: Obey flags properly.