Summary: | String::remove will convert an 8 bit string to a 16 bit string | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Michael Saboff
2012-10-03 14:16:09 PDT
The remove method is actually in WTFString.cpp (String::remove). Created attachment 166981 [details]
Patch
Comment on attachment 166981 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166981&action=review > Source/WTF/wtf/text/WTFString.cpp:281 > if (static_cast<unsigned>(lengthToRemove) > length() - position) > lengthToRemove = length() - position; No idea if that case is common. If it is, shouldn't we just return a substring sharing impl here? > Source/WTF/wtf/text/WTFString.cpp:289 > + RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(length() - lengthToRemove, data); > + memcpy(data, characters8(), position * sizeof(LChar)); > + memcpy(data + position, characters8() + position + lengthToRemove, > + (length() - lengthToRemove - position) * sizeof(LChar)); > + m_impl = newImpl.release(); I would do an inline template with that code taking CharacterType as argument. (In reply to comment #3) > (From update of attachment 166981 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166981&action=review > > > Source/WTF/wtf/text/WTFString.cpp:281 > > if (static_cast<unsigned>(lengthToRemove) > length() - position) > > lengthToRemove = length() - position; > > No idea if that case is common. > If it is, shouldn't we just return a substring sharing impl here? From what I've seen, the use of remove is within length range and this wouldn't help. > > Source/WTF/wtf/text/WTFString.cpp:289 > > + RefPtr<StringImpl> newImpl = StringImpl::createUninitialized(length() - lengthToRemove, data); > > + memcpy(data, characters8(), position * sizeof(LChar)); > > + memcpy(data + position, characters8() + position + lengthToRemove, > > + (length() - lengthToRemove - position) * sizeof(LChar)); > > + m_impl = newImpl.release(); > > I would do an inline template with that code taking CharacterType as argument. Done. Committed r130404: <http://trac.webkit.org/changeset/130404> |