Bug 26875 - Large buffer allocation on the stack
Summary: Large buffer allocation on the stack
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-30 20:55 PDT by Kent Tamura
Modified: 2009-07-04 07:06 PDT (History)
0 users

See Also:


Attachments
Proposed patch (2.09 KB, patch)
2009-06-30 20:59 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2009-06-30 20:55:31 PDT
WebCore/platform/text/TextCodecICU.cpp defines 16KB-32KB local variables.  They are too large for the stack.
Comment 1 Kent Tamura 2009-06-30 20:59:38 PDT
Created attachment 32109 [details]
Proposed patch
Comment 2 Mark Rowe (bdash) 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.
Comment 3 Kent Tamura 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?

Comment 4 Darin Adler 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.
Comment 5 Kent Tamura 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.
Comment 6 Maciej Stachowiak 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.