[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#6758) Cleaning up SlapReply usage



Mostly done with the easily spotted cases.  Hopefully HEAD's code
stayed valid through my commits.  Need to think some more for the
rest.  The decisions with greater chance of breaking things
remain, like ITS#6763.

Other issues:

* hyc, please have a look at slapd/bconfig.c 1.419.  If I have
  gotten things wrong, that one summarizes my mistakes nicely:-)

* If slap_send_ldap_result() or send_ldap_disconnect() see an entry
  with REP_ENTRY_MUST<RELEASE/BEFREED> and REP_SEARCH/SEARCHREF,
  should release/free the entry?  That helps code which sends error
  without freeing sr_entry.  It breaks if the entry comes from
  another Operation, or if the caller reuses a Search SlapReply for
  a non-Search and trusts rs->sr_un.sru_search to stay untouched.

* Is the code getting clean enough that slap_send_search_entry's
  cleanup code can flush the entry independently of whether
  op->o_tag == LDAP_REQ_SEARCH?

* slapi/slapi_pblock.c 1.81:
	/* TODO: Should REP_ENTRY_MODIFIABLE be set? */
  I've no idea, and I've more pressing things to look at...

Notes on the updates:

* "Reset some SlapReply flags & data" commits vs "avoid SlapReply reuse"
  commits:  The combination might be redundant/overcautious, but tries
  to catch problems from remaining unclean code from both ends.

* result.c, freeing as well as releasing entries as soon as possible:
  More readable, and since the entry isn't needed: The sooner we get
  rid of it, the less chance for an entry leak.

* commits "Use rs_*() to manage SlapReply entries."

  Switching to rs_flush_entry():
  - result.c:slap_send_search_entry(), overlays/pcache.c,
      overlays/translucent.c, slapi/slapi_pblock.c:
    These did not clear the MODIFIABLE flag along with MUSTBEFREED.

  Switching to use rs_ensure_entry_modifiable():
  - syncprov_operational(), overlay collect:
    noop change (except operational should not update the entry)
  - overlay valsort:
    It needlessly required REP_ENTRY_MUSTBEFREED as well as MODIFIABLE.
    But the entry can also be MODIFIABLE and e.g. on the stack.
  - contrib overlays cloak, usn:
    Had entry leaks.

-- 
Hallvard