If the stack is too deep we could get Stack Overflow in pdf's ContentEntry's destructor.

If the stack is too deep we could get Stack Overflow in pdf's ContentEntry's destructor.
Committed: https://code.google.com/p/skia/source/detail?r=7605

[Steve VanDeBogart, 2013-02-05 23:00:50]

LGTM  Do you know why the stack gets so big though? I can understand the depth
of the stack, but why is each stack frame so large that it blows out the stack.

[edisonn, 2013-02-05 23:10:45]

On 2013/02/05 23:00:50, Steve VanDeBogart wrote:
> LGTM  Do you know why the stack gets so big though? I can understand the depth
> of the stack, but why is each stack frame so large that it blows out the
stack.

in a 'recursive' call of a destructor from ~ContentEntry() to ~ContentEntry() we
use  280 bytes ... I don't know why exactly, but I think the scalar delete takes
most of the space because it dumps the whole struct on stack ... but I don't
have time to disassemble now

 	render_pdfs.exe!ContentEntry::~ContentEntry()  Line 502 + 0xe bytes	C++
 	render_pdfs.exe!ContentEntry::`scalar deleting destructor'()  + 0x14 bytes	C++
 	render_pdfs.exe!SkTScopedPtr<ContentEntry>::~SkTScopedPtr<ContentEntry>() 
Line 30 + 0x1e bytes	C++
 	render_pdfs.exe!ContentEntry::~ContentEntry()  Line 502 + 0xe bytes	C++

[Steve VanDeBogart, 2013-02-05 23:15:59]

Hmm, ok, LGTM

[mike3, 2013-02-06 03:40:50]

This while-loop is much better than a recursive sequence of destructors. Good
fix.

https://codereview.appspot.com/7313045/diff/2001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.appspot.com/7313045/diff/2001/src/pdf/SkPDFDevice.cpp#newc...
src/pdf/SkPDFDevice.cpp:502: ContentEntry* val = fNext.release();
Don't think you need this if-block, as the while-loop will perform the same
check.

[edisonn, 2013-02-06 13:39:20]

https://codereview.appspot.com/7313045/diff/2001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.appspot.com/7313045/diff/2001/src/pdf/SkPDFDevice.cpp#newc...
src/pdf/SkPDFDevice.cpp:502: ContentEntry* val = fNext.release();
agree, I will check in once the tree is opened
On 2013/02/06 03:40:50, mike3 wrote:
> Don't think you need this if-block, as the while-loop will perform the same
> check.