26875 2009-06-30 20:55:31 -0700 Large buffer allocation on the stack 2009-07-04 07:06:40 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED WONTFIX P2 Minor --- 0 tkent webkit-unassigned oldest_to_newest 129248 0 tkent 2009-06-30 20:55:31 -0700 WebCore/platform/text/TextCodecICU.cpp defines 16KB-32KB local variables. They are too large for the stack. 129249 1 32109 tkent 2009-06-30 20:59:38 -0700 Created attachment 32109 Proposed patch 129275 2 mrowe 2009-07-01 00:59:38 -0700 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. 129277 3 tkent 2009-07-01 01:25:53 -0700 (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? 129651 4 32109 darin 2009-07-02 18:54:21 -0700 Comment on attachment 32109 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. 129657 5 tkent 2009-07-02 21:18:16 -0700 (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. 129781 6 32109 mjs 2009-07-04 07:06:40 -0700 Comment on attachment 32109 Proposed patch Clearing review flag and marking obsolete, since the patch is withdrawn. 32109 2009-06-30 20:59:38 -0700 2009-07-04 07:06:40 -0700 Proposed patch use-stack.diff text/plain 2136 tkent SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NTQyMCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMDktMDYtMzAgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0u b3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEF2 b2lkIHRvIGFsbG9jYXRlIGxhcmdlIGJ1ZmZlcnMgb24gdGhlIHN0YWNrLgorCisgICAgICAgICog cGxhdGZvcm0vdGV4dC9UZXh0Q29kZWNJQ1UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4dENv ZGVjSUNVOjpkZWNvZGUpOgorICAgICAgICAoV2ViQ29yZTo6VGV4dENvZGVjSUNVOjplbmNvZGUp OgorCiAyMDA5LTA2LTMwICBNYWNpZWogU3RhY2hvd2lhayAgPG1qc0BhcHBsZS5jb20+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgU2FtIFdlaW5pZy4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vdGV4 dC9UZXh0Q29kZWNJQ1UuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vdGV4dC9U ZXh0Q29kZWNJQ1UuY3BwCShyZXZpc2lvbiA0NTQwMykKKysrIFdlYkNvcmUvcGxhdGZvcm0vdGV4 dC9UZXh0Q29kZWNJQ1UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zMDYsNyArMzA2LDcgQEAgU3Ry aW5nIFRleHRDb2RlY0lDVTo6ZGVjb2RlKGNvbnN0IGNoYXIqIAogCiAgICAgVmVjdG9yPFVDaGFy PiByZXN1bHQ7CiAKLSAgICBVQ2hhciBidWZmZXJbQ29udmVyc2lvbkJ1ZmZlclNpemVdOworICAg IFVDaGFyKiBidWZmZXIgPSBuZXcgVUNoYXJbQ29udmVyc2lvbkJ1ZmZlclNpemVdOwogICAgIFVD aGFyKiBidWZmZXJMaW1pdCA9IGJ1ZmZlciArIENvbnZlcnNpb25CdWZmZXJTaXplOwogICAgIGNv bnN0IGNoYXIqIHNvdXJjZSA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y29uc3QgY2hhcio+KGJ5dGVzKTsK ICAgICBjb25zdCBjaGFyKiBzb3VyY2VMaW1pdCA9IHNvdXJjZSArIGxlbmd0aDsKQEAgLTMyNSw2 ICszMjUsNyBAQCBTdHJpbmcgVGV4dENvZGVjSUNVOjpkZWNvZGUoY29uc3QgY2hhciogCiAgICAg ICAgIH0gd2hpbGUgKHNvdXJjZSA8IHNvdXJjZUxpbWl0KTsKICAgICAgICAgc2F3RXJyb3IgPSB0 cnVlOwogICAgIH0KKyAgICBkZWxldGVbXSBidWZmZXI7CiAKICAgICBTdHJpbmcgcmVzdWx0U3Ry aW5nID0gU3RyaW5nOjphZG9wdChyZXN1bHQpOwogCkBAIC00NTQsMTAgKzQ1NSwxMCBAQCBDU3Ry aW5nIFRleHRDb2RlY0lDVTo6ZW5jb2RlKGNvbnN0IFVDaGFyCiAKICAgICBWZWN0b3I8Y2hhcj4g cmVzdWx0OwogICAgIHNpemVfdCBzaXplID0gMDsKKyAgICBjaGFyKiBidWZmZXIgPSBuZXcgY2hh cltDb252ZXJzaW9uQnVmZmVyU2l6ZV07CisgICAgY2hhciogdGFyZ2V0TGltaXQgPSBidWZmZXIg KyBDb252ZXJzaW9uQnVmZmVyU2l6ZTsKICAgICBkbyB7Ci0gICAgICAgIGNoYXIgYnVmZmVyW0Nv bnZlcnNpb25CdWZmZXJTaXplXTsKICAgICAgICAgY2hhciogdGFyZ2V0ID0gYnVmZmVyOwotICAg ICAgICBjaGFyKiB0YXJnZXRMaW1pdCA9IHRhcmdldCArIENvbnZlcnNpb25CdWZmZXJTaXplOwog ICAgICAgICBlcnIgPSBVX1pFUk9fRVJST1I7CiAgICAgICAgIHVjbnZfZnJvbVVuaWNvZGUobV9j b252ZXJ0ZXJJQ1UsICZ0YXJnZXQsIHRhcmdldExpbWl0LCAmc291cmNlLCBzb3VyY2VMaW1pdCwg MCwgdHJ1ZSwgJmVycik7CiAgICAgICAgIHNpemVfdCBjb3VudCA9IHRhcmdldCAtIGJ1ZmZlcjsK QEAgLTQ2NSw2ICs0NjYsNyBAQCBDU3RyaW5nIFRleHRDb2RlY0lDVTo6ZW5jb2RlKGNvbnN0IFVD aGFyCiAgICAgICAgIG1lbWNweShyZXN1bHQuZGF0YSgpICsgc2l6ZSwgYnVmZmVyLCBjb3VudCk7 CiAgICAgICAgIHNpemUgKz0gY291bnQ7CiAgICAgfSB3aGlsZSAoZXJyID09IFVfQlVGRkVSX09W RVJGTE9XX0VSUk9SKTsKKyAgICBkZWxldGVbXSBidWZmZXI7CiAKICAgICByZXR1cm4gQ1N0cmlu ZyhyZXN1bHQuZGF0YSgpLCBzaXplKTsKIH0K