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 apr_mmap_t.
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 object
* 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.
--Brian
