Use macros for new and delete in SkTemplates.h

Use macros for new and delete in SkTemplates.h

Remove use of new placement array.

R=bungeman@google.com, reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9329

[bungeman-skia, 2013-05-25 03:22:07]

While reviewing this change I discovered yet another dark corner of c++ with
which I was not previously familiar.

https://codereview.chromium.org/15739013/diff/1/include/core/SkTemplates.h
File include/core/SkTemplates.h (left):

https://codereview.chromium.org/15739013/diff/1/include/core/SkTemplates.h#ol...
include/core/SkTemplates.h:232: fArray = new (fStorage) T[count];
I think this is a bug. You can never be sure how much space an operator new[]
will take up, so you can't portably use placement operator new[]. See
http://stackoverflow.com/questions/8720425/array-placement-new-requires-unspe...
for one discussion, but basically 5.3.4.12 [expr.new] says operator new[] is
allowed to allocate a 'cookie' before the returned pointer so that operator
delete[] knows how many elements to delete. Basically, we need an assertion here
that after this assignment fArray == fStorage, but I bet this fails somewhere.
We're probably randomly overwriting bits of the stack depending on how we use
this class. See also
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#476 for the current
official position.

As such, I am against the change to add SkNEW_PLACEMENT_ARRAY (as there is no
safe way to use it) and in addition we need to fix this code to not do this.

[reed1, 2013-05-28 12:39:08]

Good catch about placement_new[]. I think we should rewrite SkAutoSTArray to
always perform a manual loop for constructors/destructors (calling malloc if we
need more space than N), and we should just remove our placement new[] macro.

[scroggo, 2013-05-29 19:47:49]

On 2013/05/28 12:39:08, reed1 wrote:
> Good catch about placement_new[]. I think we should rewrite SkAutoSTArray to
> always perform a manual loop for constructors/destructors (calling malloc if
we
> need more space than N), and we should just remove our placement new[] macro.

Done.

[reed1, 2013-05-29 19:51:26]

lgtm

[bungeman-skia, 2013-05-29 20:06:57]

lgtm

[scroggo, 2013-05-29 20:10:57]

Committed patchset #2 manually as r9329 (presubmit successful).