Pierangelo Masarati wrote:
I think you misinderstand. The new code I have committed does use a statically defined array of controls, but the indices are not compile-time constants, they are generated at runtime. All controls that slapd knows about are registered via register_supported_control(). All controls that are dynamically loaded are registered this way as well. Each control is now assigned a CID when it is registered, and the CIDs of the well-known controls are saved in the slap_cids structure. So there is very easy, direct access to all controls, whether they were statically compiled in or dynamically loaded. The only open question was whether to make the array of registered controls static or dynamic, and for ease of implementation I went with static.Luke Howard wrote:
Yes, that was my point. I was talking about an array whose size is setMaybe I was too cryptical; of course, any time a new control gets supported, NID_MAX is pushed to a higher value; that should really read (MAX_DEFINED_NID+1).
SLAPI plugins (and perhaps OpenLDAP ones?) can register controls dynamically.
equal to the number of currently registered controls, keeping in mind
that controls can be registered dynamically. But the code I committed to
HEAD just uses a constant size (SLAP_MAX_CIDS) which I've arbitrarily
set at 32 for now. For reference, there are 15 already hardcoded in
slapd/controls.c, and the password policy overlay registers one as well.
Howard,
I understand Luke's issue, and we really should not inhibit the dynamic
registration of new controls; my understanding of your design, however,
was that direct access to a well known and registered in advance control
would be made very easy by directly pointing to its index in a statically
defined array of controls, and indices would use constants, or macros, or
enum members, for the sake of readability and generality. Dynamically
registered controls are likely to break this, maybe we could just treat
them separately, e.g. in a list or a dynamically allocated array, in any
case in a separate container with a well-defined access API to both
register, check presence and use dynamically registered controls. I don't
like much this idea of having two different storages for the same stuff,
but I liked the initial idea of having very easy and direct access to
well-known, hardcoded controls.
This is definitely a growing problem. I'm not sure yet what would be good here. There is already a connection_fake_init() for dummy Connection structures, I guess we need something similar for Operations.Code that creates dummy Operation structs (such as in
sasl.c/saslauthz.c) might need updating to create a valid struct if we
went with the purely dynamic approach I described before, so sticking to
a constant made the patch simpler.
About creating dummy Operation structures, since the Operation structure
has been growing recently due to many syncrepl related stuff, control
related stuff and so, sometimes (e.g. in overlays, but also in some
internal code that exploits the callback mechanism e.g. in ACLs or authz
related stuff) I feel uncomfortable about initializing the structure. Usually I see two approaches: "from scratch", i.e. "Operation op2 = {0};"
which requires one to set many fields by simply doing the "op2.o_ndn =
op->o_ndn" thing, and "carve out", i.e. "Operation op2 = *op;" followed
op2.ors_tlimit = SLAP_NO_LIMIT;" and so on which may be tricky if the new
operation is going to be the same REQ type of the old one, or because in
many cases we cannot assume the search will not pass thru overlays that,
sya, will rewite the o_req_ndn or change the ors_attrs and so. Any
thoughts about a clean way to initialize new Operation structures, e.g. by
blocks?
-- -- Howard Chu Chief Architect, Symas Corp. Director, Highland Sun http://www.symas.com http://highlandsun.com/hyc Symas: Premier OpenSource Development and Support