Re: [PATCH] Reinstate Mempools useage for MemBlob

On Tue, Apr 5, 2011 at 10:22 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> 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.

Done.

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

Yes. Done.

>> -#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);

mem must be non-null, or MemBlob will throw in the constructor (or in
any reallocation call).
I'll leave the guard in anyways, even though I am certain it will never be hit.

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

Committing shortly.

Thanks.

-- 
    /kinkie