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

Re: (ITS#5340) REP_ENTRY_MODIFIABLE bug in dynlist

ando@sys-net.it writes:
> That's why I'm not so happy with MUSTRELEASE.

Seems to be required though.  cvs log for slap.h revision 1.678.

But it does look to me like SlapReply needs a BackendDB *e_owner member.
I haven't observed buggy behavior related to that (or tried to provoke
any), but it looks hairy for a backend with a be_release() function to
make sure to intercept every entry from elsewhere and record previous
owner info in e_private or callbacks.

> OK, makes sense.  But perhaps creating entries on the stack could be
> deprecated now that entries are pooled and reused by calling
> entry_alloc() entry_free().  This is just food for thoughts, we could as
> well leave all those flags 'round, if we can streamline their handling.

I think we might as well leave them.  Since most are for saving memory,
might even want to expand them eventually.  (E.g. separate flags for
_which_ parts of an entry are modifiable.  Modifiable attribute list,
DNs, values _in_ attribute list, etc.)

For that matter, looking at entry_free/entry_alloc, I don't see how that
pooling saves more than it costs.  It saves 1 malloc for the Entry
structure, but not the mallocs for the DN and attribute list.  And it
costs two global mutex lock/unlocks per entry, in entry_alloc() and
entry_free().  An entry on the stack avoids both malloc and locks.

Anyway, I guess one conclusion of all this is that the code really is as
sloppy as it looked, there are no magic saving throws in there.  (Except
with finding the backend for be_release if I'm still missing something
about that.)