Re: (ITS#6758) Cleaning up SlapReply usage

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

In addition to the issues listed in my last message, this ITS depends
on these:
- ITS#6763: slapd does not clear sr_entry/ctrls on sizelimit/busy
- ITS#6776: Control handling in PassMod exop + ppolicy
- ITS#6760: rwm broken entry handling

====

Howard Chu writes:
>h.b.furuseth@usit.uio.no wrote:
>> * hyc, please have a look at slapd/bconfig.c 1.419.  If I have
>>    gotten things wrong, that one summarizes my mistakes nicely:-)

I meant to say "If I've broken anything, that commit should summarize
my worst mistakes."  At least that worry is out of the way:

> I think you're over-paranoid. config_build_entry() is only called by
> internal plumbing and the SlapReply here never gets sent to
> send_ldap_result(). It's only passed in because be_add() expects one;
> the contents are mostly irrelevant. (Aside from rs_err)

Thanks.  Not exactly what I hoped to hear, but I did ask...  I can
revert it, or since this SlapReply is so uninteresting, maybe allow rs
= NULL in this case so some callers can omit it.

====

Summary of changes:

* Add rs_assert_*(), disabled by default, to check SlapReply state.

* Majority of changes:  Avoid reusing SlapReply: Reinitialize these
  variables at need with rs_reinit() or move a declaration inwards
  (typically into loops).  I think I introduced 2 new SlapReply's,
  in bconfig.c (unnecessarily as Howards says) ad in pcache.

* Use the rs_*() functions in result.c to manage SlapReply entries,
  rather than freeing/releasing by hand and every so often forgetting
  to handle some of the REP_ENTRY_* flags.

  This also makes slapd more aggressive about freeing entries.  E.g.
  result.c _released_ entries at need before sending, so entry locks
  for cached entries would not wait for network traffic.  OTOH it did
  not at that point bother with entries that merely needed _freeing_.

* "Reset dangerous REP_ENTRY_* flags."

  This is so when a module puts e.g. puts Extended Operation data
  in the SlapReply, another module won't try to entry_free/release()
  that non-entry because of some leftover REP_ENTRY_* flags.

  This may have been over-paranoid too:  I seem closer to having
  rs.<sr_type, sr_un> behave like a variant record where rs.sr_type
  actually can be trusted.  Anyway, this paranoia shouldn't hurt.

Others are some minor cleanups:

* Reset some SlapReply flags & data.  Reset data more consistenlty.
  (E.g. rs_entry and flags go together, so don't reset just entry.)

* Clear out or avoid filling in known unwanted info (Don't set sr_err
  = non-success before entering an operation handler, do not set
  sr_text with sr_err = LDAP_SUCCESS, etc.)

* Add an error check or two (was actually done for consistency sr_err
  vs sr_text)

* slap.h: Cast REP_* #defines to slap_mask_t

* No-op changes (remove lint, rearrange/simplify code a bit, etc)

-- 
Hallvard