Re: util_ldap [Bug 29217] - Remove references to calloc() and free()

[Brad Nicholes <bn...@novell.com>]

   I have considered using a pool per entry in other caching code that I
have written just so that I could have much finer control over
allocating and freeing the memory.  But in the end what it really comes
down to is malloc and free and if that is what you really want, then why
not just use malloc and free.  Pools just add a layer of memory
management between your application and the actual allocation that may
be of no use.  You have simply replaced malloc() with apr_pool_create()
and free() with apr_pool_destroy().  If you forget to call
apr_pool_destroy(), you have a memory leak in the same way as you would
if you forget to call free().  When the application shuts down, the pool
management will clean up the memory but then so will the process
management of the OS.  Outside of some debugging help during
development, converting to pools and reslists is just adding a lot of
unneeded overhead.  The advantage to using memory pools is in situations
where you need a pool of memory that can created, pieced out during a
specific operation and then completely cleared once the operation is
complete and reused without having to recreate it.  Global caches just
don't lend themselves to this model.  The pieces can come can go, but
you can never really clear the whole thing out and reuse it because the
operation never actually ends.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> Graham Leggett <mi...@sharp.fm> Thursday, June 10, 2004 9:46:16
AM >>>
Brad Nicholes wrote:

>    At least on NetWare, switching to pools would make the code much
> more complex.  Rather than simply calling calloc and free in the
same
> way that we are calling apr_rmm_calloc() and apr_rmm_free(), we
would
> have to implement essentially the same model using pools and
reslists. 
> It seems to me like using a sledge hammer to drive a finishing nail
in
> this instance.  Also in the end, it all boils down to malloc and
free
> anyway.  As far as debugging goes, I can understand why it might be
> easier using the pool debug code, but we have never been successful
in
> making the pool debug code work on NetWare.  Granted we probably
haven't
> tried real hard mainly because NetWare already has some good memory
> debugging capabilities built into the OS.  If debugging is a problem,
I
> think it might be easier to implement some memory debugging code
> specifically for the LDAP_cache rather than trying to retrofit it
with
> pools and reslists.

The theory is that the pools code is already hopefully been 
pre-debugged, you can allocate memory from a pool, and be reasonably 
sure that the problems of freeing the memory is handled for you. (If 
this is not the case, it won't be an LDAP bug, but an Apache wide bug).

The only real issue really is worrying about the scope of the pool.

Thinking further about this, we could use one pool per cache entry. To

delete that cache entry just means to destroy the pool. No more need to

walk the cache entry and delete each buffer one by one, and no room to

make mistakes. No more chance that somebody adds a field to the cache 
entry, and then forgets the code to free the cache entry.

Creation of the pool would only be done on creation of the cache entry,

which in turn is done only on the first time this user is
authenticated, 
all further requests being cached, so it doesn't seem to be expensive 
either, unless someone with a better understanding of the internals of

pools would be able to say whether this idea is good or bad.

Regards,
Graham
--