RESOLVED WONTFIX 26875
Large buffer allocation on the stack 
https://bugs.webkit.org/show_bug.cgi?id=26875
Summary Large buffer allocation on the stack
Kent Tamura
Reported 2009-06-30 20:55:31 PDT
WebCore/platform/text/TextCodecICU.cpp defines 16KB-32KB local variables. They are too large for the stack.
Attachments
Proposed patch (2.09 KB, patch)
2009-06-30 20:59 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2009-06-30 20:59:38 PDT
Created attachment 32109 [details] Proposed patch
Mark Rowe (bdash)
Comment 2 2009-07-01 00:59:38 PDT
In what sense are they too large for the stack? Rather than manually calling delete it would be preferable to use an OwnPtr to make the code more robust against future modifications.
Kent Tamura
Comment 3 2009-07-01 01:25:53 PDT
(In reply to comment #2) > In what sense are they too large for the stack? It's almost subjective. Chromium doesn't have any problems about this "large" buffers. However Coverity Prevent tool detected it as defects and they might make problems on other platforms. If you think we don't need to fix them, it's ok to reject the patch. > Rather than manually calling delete it would be preferable to use an OwnPtr to > make the code more robust against future modifications. Dows OwnPtr work for arrays?
Darin Adler
Comment 4 2009-07-02 18:54:21 PDT
Comment on attachment 32109 [details] Proposed patch The whole point here is to use a buffer size that fits on the stack to avoid the overhead of heap allocation. If some platforms need the conversion chunk to be smaller, it seems best to me to simply make ConversionBufferSize a smaller number for those platforms.
Kent Tamura
Comment 5 2009-07-02 21:18:16 PDT
(In reply to comment #4) > it seems best to me to simply make ConversionBufferSize a smaller > number for those platforms. That's reasonable. Ok, I withdraw this patch and close this bug.
Maciej Stachowiak
Comment 6 2009-07-04 07:06:40 PDT
Comment on attachment 32109 [details] Proposed patch Clearing review flag and marking obsolete, since the patch is withdrawn.
Note You need to log in before you can comment on or make changes to this bug.