callback functionality for dyanmic memory allocation

Jonathan Larmour jifl@eCosCentric.com
Tue Nov 5 09:08:00 GMT 2002


NavEcos wrote:
> On Tuesday 05 November 2002 01:50 am, Jonathan Larmour wrote:
> 
>>
>>Well, okay you asked for it ;-). I'm not convinced of the need to have the
>>callback functions being a zillion different types, most of which are
>>pretty similar (pool, ptr, size most times).
>>
>>I think it may make for a more understandable interface if it's a single
>>type, with an "operator" enum indicating what function is being performed.
>>I would even go as far as suggesting, since the primary use of this is
>>debugging, that there even only be one function that's called full stop.
>>i.e. instead of 8 statics, there's just the one. If the user's callback
>>isn't interested in a particular operation, it can just drop out and
>>ignore it.
>>
>>This callback could be of type (e.g. for dlmalloc):
>>
>>typedef void Cyg_mempool_dl_callback_fn( enum Cyg_Mempool_dl_op op,
>>Cyg_Mempool_dlmalloc *pool, const cyg_uint8 *base_ptr, cyg_int32 size,
>>cyg_int32 *newsize );
> 
> 
> OK - no big deal although you'll need parameters for the allocated
> pointer and also an "original size" for realloc.  That should cover
> it I think.

I think you should be able to "overload" some of the existing parameters. 
e.g. the original size for realloc could go to "size" rather than a new 
parameter. And indeed if we use cyg_uint8 **ptr instead of const cyg_uint8 
*base_ptr that's similarly more flexible.

>>Obviously, unused arguments for the particular op can just be ignored.
>>
>>I think this would greatly simplify this for users. Having so many
>>different callbacks and types seems cumbersome.
>>
>>I'm not clear on why for e.g. fn_pre_realloc_cb you want to take the
>>_address_ of alloc_ptr. alloc_ptr itself should do. Ditto for other
>>cyg_uint8**.
> 
> 
> To modify the pointer.  Say you want to put a guard band at the
> beginning of the allocated block of memory for example.

Ahhh... that's why you also pass &size, to allow that to be changed as 
well to allow room for such a band. I see.

>>This is definitely large enough for a copyright assignment, so that's
>>required before it can be included.
> 
> 
> No problem.  I'll get that into the mail tomorrow.  I have authority
> to sign documents for my company.

Cool. You'll only need this one to cover all future contributions you wish 
to make too so it's one time pain.

>>Just when it comes to code layout, I think it's superior that instead of
[snip]
>>we have:
>>
>>     cyg_uint8 *pRet;
>>
>>#ifdef CYGSEM_MEMALLOC_ALLOCATOR_FIXED_CALLBACK
>>     if ( fn_pre_alloc_cb != NULL ) {
>>        (*fn_pre_alloc_cb) (this);
>>     }
>>#endif
>>
>>     pRet = mypool.try_alloc( 0 );
>>
>>#ifdef CYGSEM_MEMALLOC_ALLOCATOR_FIXED_CALLBACK
>>     if ( fn_post_alloc_cb != NULL ) {
>>        (*fn_post_alloc_cb) (this, &pRet);
>>     }
>>#endif
>>     return pRet;
> 
> Either one.  I thought it was less noisy the first way, but I'm not
> religous about it.

I think "common" code should only be in one place. I just think it makes 
the intent clearer.

Thanks for putting up with my comments :-),

Jifl
-- 
eCosCentric       http://www.eCosCentric.com/       <info@eCosCentric.com>
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine



More information about the Ecos-patches mailing list