Bug 99230 - StringPrototype::jsSpliceSubstringsWithSeparators() doesn't optimally handle 8 bit strings
Summary: StringPrototype::jsSpliceSubstringsWithSeparators() doesn't optimally handle ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-12 18:33 PDT by Michael Saboff
Modified: 2012-10-17 14:54 PDT (History)
1 user (show)

See Also:


Attachments
Patch (2.27 KB, patch)
2012-10-12 18:36 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2012-10-12 18:33:00 PDT
jsSpliceSubstringsWithSeparators() handles the case where both the source an separators are all 8 bit strings, but doesn't handle when they are mixed sizes.
Comment 1 Michael Saboff 2012-10-12 18:36:18 PDT
Created attachment 168528 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-10-15 12:51:33 PDT
Comment on attachment 168528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168528&action=review

> Source/JavaScriptCore/runtime/StringPrototype.cpp:404
> +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters8(), sepLen);
> +                else
> +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters16(), sepLen);

Why not just have a StringImpl::copyChars() which takes a StringImpl and does this if internally?
Comment 3 Michael Saboff 2012-10-16 16:17:52 PDT
(In reply to comment #2)
> (From update of attachment 168528 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168528&action=review
> 
> > Source/JavaScriptCore/runtime/StringPrototype.cpp:404
> > +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters8(), sepLen);
> > +                else
> > +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters16(), sepLen);
> 
> Why not just have a StringImpl::copyChars() which takes a StringImpl and does this if internally?

We would likely need a few flavors.  This flavor would use the string length, but at least another one would need a string offset and a reduced length.
Comment 4 Geoffrey Garen 2012-10-17 14:48:19 PDT
Comment on attachment 168528 [details]
Patch

r=me
Comment 5 WebKit Review Bot 2012-10-17 14:54:47 PDT
Comment on attachment 168528 [details]
Patch

Clearing flags on attachment: 168528

Committed r131648: <http://trac.webkit.org/changeset/131648>
Comment 6 WebKit Review Bot 2012-10-17 14:54:51 PDT
All reviewed patches have been landed.  Closing bug.