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

Re: dynamic groups

MichaÅ SzulczyÅski wrote: 
Hello,

I'm attaching the "Really dynamic list" overlay, on which I have been
working for the past 2 weeks. This is the Technology Preview version, so
I need your input on the implementation. Also feel free to criticize, or
to point out the flaws in my understanding of OpenLDAP, or the
implementation of this overlay.


Nice job, getting to working code.

You don't need to use config_generic_wrapper in this overlay, that's primarily for backward compatibility with nested databases. Fully dynamic modules just need to set bi_cf_ocs and leave bi_db_config unset.

I think the amount of comments is too few. It takes more than one read thru the code to understand what an rdynlist_entry_t is used for; this should have been noted in a comment. Likewise for rdynlist_filter_t.

In rdynlist_search_cb you shouldn't free rs->sr_entry yourself, that's the frontend's job. Calling entry_free() directly is usually the wrong thing to do anyway; you should use be_entry_release_rw or overlay_entry_release_ov.

In rdynlist_add_group() not sure what your "TODO check if alloc was successful" comment is about. The ch_* routines never return on failure, they simply abort. As such, it's unnecessary to check for success. You can of course use ber_memalloc directly and check for success/failure if you wish but there's usually no good reason to do so. I.e., once the server starts running out of memory, it's pretty much dead anyway.

Looks like you leak the URL descriptor on a normal run thru this function.

Again in rdynlist_group_add_cb don't free rs_sr_entry yourself.

In rdynlist_response() you call attrs_find to obtain an entry's objectclass (and other attributes) and then immediately release the entry. This is not thread-safe; once the entry is released it is possible for it to disappear (due to other operations cycling the backend's entry cache, if any. etc...). At that point the attribute pointer will be invalid. Either dup the attribute first, or don't release the entry until you're done with the attribute.

You should call build_new_dn() with the current op->o_tmpmemctx and use op->o_tmpfree() instead of ch_free since new_dn is just a temporary value.

Same entry_get/entry_release problems in rdynlist_modify_entry().

You have mismatched functionality in rdynlist_db_open/rdynlist_db_destroy. I.e., what you allocate in db_open should be freed in db_close, not db_destroy.

I have tested it, and it works without problems (at least on my setup).

Try it with a database with about 1 million users, with 900,000 or so members of a dynamic group.

While it's good that you got this code working, I still believe you've completely missed the point of dynamic groups.

This overlay works by updating the dynamic list entry's member attribute
(which is not modifiable by the user) on an add/delete/modify/modrdn
operation, adding or deleting the updated entry's DN to/from the dynamic
list when the entry matches the any of the memberURL filters of the
dynamic list.

I have modified the dyngroup schema slightly, adding the 'member'
attribute to the MAY clause. This is needed to store the 'materialized'
member DN's in the dynamic list entry in the database.

The config is similar to the dynlist overlay, but with mandatory member
attribute:

rdynlist-attrset <group-oc> <URL-ad> <member-ad>



* rdynlist.patch is the patch for the schema and the
configure/makefiles.
--
MichaÅ SzulczyÅski
Praktykant
Altkom Akademia S.A. http://www.altkom.pl
Warszawa, ul. ChÅodna 51


SÄd Rejonowy dla m.st. Warszawy w Warszawie, XII WydziaÅ Gospodarczy Krajowego Rejestru SÄdowego,
KRS: 0000120139, NIP 118-00-08-391, KapitaÅ zakÅadowy: 1000 000 PLN. Adres rejestrowy Firmy - ul. Stawki 2, 00-193 Warszawa.
Niniejsza wiadomoÅÄ zawiera informacje zastrzeÅone i stanowiÄce tajemnicÄ przedsiÄbiorstwa firmy Altkom Akademia S.A.
Ujawnianie tych informacji osobom trzecim lub nieuprawnione wykorzystanie ich do wÅasnych celÃw jest zabronione.
JeÅeli otrzymaliÅcie PaÅstwo niniejszÄ wiadomoÅÄ omyÅkowo, prosimy o niezwÅoczne skontaktowanie siÄ z nadawcÄ oraz usuniÄcie wszelkich kopii niniejszej wiadomoÅci.
This message contains proprietary information and trade secrets of Altkom Akademia S.A. company.
Unauthorized use or disclosure of this information to any third party is prohibited.
If you received this message by mistake, please contact the sender immediately and delete all copies of this message. 



--
  -- Howard Chu
  Chief Architect, Symas Corp.  http://www.symas.com
  Director, Highland Sun        http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP     http://www.openldap.org/project/