[Date Prev][Date Next]
Re: (ITS#6758) Cleaning up SlapReply usage
In addition to the issues listed in my last message, this ITS depends
- 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:
>> * 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
* slap.h: Cast REP_* #defines to slap_mask_t
* No-op changes (remove lint, rearrange/simplify code a bit, etc)