Re: dynamic groups

Michał Szulczyński wrote: 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. 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. -- -- 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/