Bug 26875

Summary:

Large buffer allocation on the stack

Product:

WebKit

Reporter:

Kent Tamura <tkent>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Minor

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	none

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	Details Formatted Diff Diff
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.