[Webkit-unassigned] [Bug 66661] Replace some usages of Vector<UChar> to StringBuilder

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=66661





--- Comment #10 from Darin Adler <darin at apple.com>  2011-08-26 07:52:59 PST ---
(From update of attachment 105335)
View in context: https://bugs.webkit.org/attachment.cgi?id=105335&action=review

This patch has far too many changes all at once. This could and should be done in smaller pieces. Adding lots of new functions and using them in lots of new places all in one patch is unnecessary for a refactoring patch. Especially because these functions do pointer arithmetic. We need to carefully think about overflow in all the new code.

A good approach is to start with a large patch like this one in your tree and try to do only the parts of it that are obviously correct and easy to review. That first patch could have a lot of changes, but all super-straightforward ones. Then the trickier parts, such as adding new functions, can be done carefully with one function at a time.

I’d suggest posting a first patch that uses StringBuilder more in various places where that can be done without even modifying it, for example.

> Source/JavaScriptCore/JavaScriptCore.order:-326
> -__ZN3WTF13StringBuilder6appendEPKtj

There is no need to try to update the order file. There is no harm having old functions in there, and no benefit to trying to update it manually. It will be regenerated eventually.

> Source/JavaScriptCore/wtf/text/StringBuilder.cpp:126
> +        memcpy(m_bufferCharacters + insertPosition + insertLength, currentCharacters + insertPosition, static_cast<size_t>(m_length - insertPosition) * sizeof(UChar));

Adding insertPosition to insertLength can lead to overflow. This code has to be written in a way that deals with that possibility.

> Source/JavaScriptCore/wtf/text/StringBuilder.cpp:152
> +                memmove(m_bufferCharacters + position + length, m_bufferCharacters + position, static_cast<size_t>(m_length - position) * sizeof(UChar));

Adding position to length can lead to overflow. This code has to be written in a way that deals with that possibility.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.