[squid-dev] [PATCH] turn mempools' delete(nullptr) into NOP

[Alex Rousskov]

On 09/29/2015 01:33 PM, Kinkie wrote:
> Hi,
>   while checking the mempools code during the discussion of thread
> "[PATCH] Refactor htcpDetail into MEMPROXY_CLASS" I found out that
> mempools' operator delete has a different behavior than that of
> ::delete.
> In particular, ::delete(nullptr) is defined to be a NOP [1]. Our code
> instead aborts:

I agree that this should be fixed/changed.


> MEMPROXY_CLASS's delete calls MemAllocatorProxy::freeOne which calls
> MemImplementingAllocator::freeOne (via MemAllocator) which asserts on
> nullptr.
> 
> The assert is needed because which freeOne calls
> MemAllocator::deallocate which is virtual, specialized by
> MemPoolChunked and MemPoolMalloc, neither of which can accept
> nullptr's in the general case.
> The whole setup is right now working because the API makes it a
> responsiblity of the caller never to delete(nullptr).
> 
> In the thread I mentioned above, Amos suggests to change our
> user-facing API to match ::delete. This patch does exactly that, by
> accepting but not processing nullptrs in
> MemImplementingAllocator::freeOne.
> 
> [1] http://stackoverflow.com/questions/6731331/is-it-still-safe-to-delete-nullptr-in-c0x


I believe you have correctly modified the wrong code. STL allocators
throw when fed a null pointer[2]. Our allocators should [continue to] do
the same. The logic behind STL design makes sense to me: The reasoning
behind allowing "delete null" differs a lot from the reasoning behind
allowing Allocator::deallocate(null) IMHO!

I recommend modifying MEMPROXY_CLASS delete() to comply with [1] by not
feeding the allocator object with invalid information.

[2]
http://stackoverflow.com/questions/3104543/c-allocatorxdeallocatenull-1-allowed


HTH,

Alex.