>The LDAP cache is a complex memory structure - each cache entry consists >of a struct with a whole lot of malloc'ed blocks. And there are >different kind of cache entries (bind cache, opcache). Deleting an entry >means walking through the struct and relying on a human writing code to >delete each block allocated one by one, and finally to delete the >struct. If one of the free()'s are missed, leak. > >If there was a single pool per cache entry, to remove the entry, just
>call apr_pool_destroy() once, not free() many times, and be confident >that the memory is gone and leak free. I see your point and it would be nice to just make a single apr_pool_destroy() call rather than traversing the entire node and freeing each individual structure element. But the way that the code is designed today, you might as well redesign and rewrite the entire cache_mgr because I don't see how you could retrofit it to use apr_rmm_xxx() alloc'ed memory for the shared memory case and apr_pool memory for the non-share memory case. Allocating and destroying the two types of memory are completely different and trying to maintain two versions of the cache_mgr would be a nightmare. Also, I don't believe that apr_pool supports shared memory. I could be wrong. (Sorry for the redundancy, I think we both agree on these points) The cache and cache_mgr code today seems very straight forward. If you want to destroy the url_node, you call util_ldap_url_node_free(). If you want to destroy the search_node, you call util_ldap_search_node_free(), etc. All virtual allocations and deallocations are done in either the xxx_node_copy or xxx_node_free functions and the deallocations are done in a very hierarchical fashion. The cache_mgr calls util_ldap_url_node_free() which in turn indirectly calls each of the xxx_node_free() functions. As I see it, there is little room for error. The cache_mgr itself is written in such a way that it doesn't really care what kind of memory you stuff into the payload and destroying the cache struct itself is nothing more than a free. Do we even know that there is a problem with this code? I haven't seen any memory leaks so far. I would hate to go to all of the work to redesign and rewrite the ldap_cache manager for little to no gain. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com
