apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject Re: [PATCH] Re: mmap_setaside?
Date Wed, 14 Nov 2001 16:57:11 GMT
Cliff Woolley wrote:

>On Tue, 13 Nov 2001, Brian Pane wrote:
>>Thanks for the pointer.  Here's my attempt to work around the problem
>>that you noted in that message--the fact that the apr_mmap_t cleanup
>>function isn't accessible from within mmap_setaside.  My solution was
>>to add a pointer to the cleanup function as an extra field in the
>Why a function pointer instead of just exposing mmap_cleanup as a public
>function?  You're essentially making it public anyway through the function
>pointer, and we don't really need polymorphism since there's only one
>mmap_cleanup function for a given APR platform...

Actually, the more I think about it, the less I like the idea of
exposing mmap_cleanup in any form (including the function pointer
in the struct).  The problem is that it introduces extra coupling
between the mmap code and the bucket code, so that the latter can
take advantage of an implementation detail of the former (the fact
that there happens to be exactly one cleanup function registered
per apr_mmap_t in its original pool).

As an alternate implementation, how about this?

   * Transfer all of the registered cleanup callbacks for the specified 
   * from one pool to another
   * @param src The pool that currently contains the cleanups for the object
   * @param src The pool to which the cleanups should be moved
   * @remark dst must be an ancestor of src
  apr_status_t apr_pool_cleanup_transfer(apr_pool_t *src, apr_pool_t *dst,
                                         void *object);

If that doesn't bother anybody too much, I'll implement it sometime
later today.


>One thing, though, shouldn't we be copying to the heap instead of to the
>pool?  That way we can at least free() the thing when we're done with
>it rather than growing the pools to be really huge forever...

Using the heap instead makes sense to me--especially since a sufficiently
large pool allocation will often have to alloc from the system heap
anyway.  I'll change that bit of the code to use a heap bucket in the
next iteration of this patch.


View raw message