Re: [PATCH] Reinstate Mempools useage for MemBlob

On 04/05/2011 10:10 AM, Kinkie wrote:

> +/** Given the requested minimum size, return a rounded allocation size
> * for the backing store.
> +// TODO: exploit better integration with MemPools, moving much of this there.
> +// TODO: have MemPools do system page size magic. They don't really belong here

The above comments make no sense given that the new
MemBlob::calcAllocSize() does nothing. There is no rounding, no "much of
this", and no "system page size magic" any more.

I suggest removing MemBlob::calcAllocSize() completely and moving any
still relevant comments to mem.cc.

> -#if MEMBLOB_USES_MEM_POOLS
> - //no mempools for now
> - // \todo reinstate mempools use
> memFreeString(capacity,mem);

memFreeString() does not handle deallocation of nil buffers IIRC. It is
very unlikely but still possible that the mem buffer remains nil in the
destructor if we hit an exception somewhere along the way to the mem
allocation call. Let's handle that case gracefully here or in
memFreeString():

    if (mem || capacity)
       memFreeString(capacity,mem);

I think this patch can go in after the above polishing touches.

Thank you,

Alex.
Received on Tue Apr 05 2011 - 20:22:43 MDT