Re: [PATCH] Restore mempools functionality for MemBlob

On 03/30/2011 05:58 PM, Kinkie wrote:
> + // if pools are not yet ready, make sure that
> + // the requested size is not poolable
> + if (!MemIsInitialized)
> + net_size=1+StrPoolsAttrs[mem_str_pool_count-1].obj_size;
> +

Depending on the timing of the memAllocString() call and other factors,
net_size may become 1 after the above. Under other circumstances, it may
become smaller than it was. Both outcomes would be wrong: net_size can
only go up.

> /* free buffer allocated with memAllocString() */
> void
> memFreeString(size_t size, void *buf)
> {
> - int i;
> MemAllocator *pool = NULL;
> - assert(size && buf);
> + assert(buf);
>
> - for (i = 0; i < mem_str_pool_count; ++i) {
> + for (unsigned int i = 0; i < mem_str_pool_count; ++i) {
> if (size <= StrPoolsAttrs[i].obj_size) {
> assert(size == StrPoolsAttrs[i].obj_size);
> pool = StrPools[i].pool;
> break;
> }
> }
>
> memMeterDec(StrCountMeter);
> memMeterDel(StrVolumeMeter, size);
> pool ? pool->freeOne(buf) : xfree(buf);
> }

If memFreeString() is called before StrPoolsAttrs are assigned, the
above will work, I think, but purely by luck. The code will iterate the
StrPoolsAttrs array in invalid state. And if the "size" is zero, the
code will even think that it found the right pool (but that pool will be
NULL so we get lucky again). A samilar comment applies to memAllocString().

BTW, zero-size free and allocation should be supported, IMO, and your
removal of the corresponding assert seems to agree with that.

And more whitespace changes.

HTH,

Alex.
Received on Thu Mar 31 2011 - 00:28:12 MDT