This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: callback functionality for dyanmic memory allocation


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]