[webkit-reviews] review denied: [Bug 50245] Add word-prefix search options to the text search without ICU : [Attachment 95002] word-prefix search for no-ICU code path (v1.0)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 17 11:01:26 PDT 2011
Darin Adler <darin at apple.com> has denied yi shen <yi.4.shen at nokia.com>'s
request for review:
Bug 50245: Add word-prefix search options to the text search without ICU
https://bugs.webkit.org/show_bug.cgi?id=50245
Attachment 95002: word-prefix search for no-ICU code path (v1.0)
https://bugs.webkit.org/attachment.cgi?id=95002&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=95002&action=review
Need to at least fix the memcpy problem. But I think that also using a circular
buffer instead of a memmove each time is the way to go. It may be tricky to get
a circular buffer to work with functions such as U16_GET, though, so not sure
it will be easy.
>> Source/WebCore/editing/TextIterator.cpp:2189
>> +
>
> same thing here. We can move identical code to the common place.
We don’t want to copy and paste this code, yes, so we need a version of the
patch that does not involve a second copy.
>> Source/WebCore/editing/TextIterator.cpp:2236
>> + memcpy(m_context.data(), m_context.data() + 1,
(m_context.size() - 1) * sizeof(UChar));
>
> memcpy to overlapped place may cause unexpected results?
That’s right. You can’t use memcpy for this; it has to be memmove instead.
And doing a memmove every time we add a character is really inefficient. I
suggest considering a circular buffer instead.
More information about the webkit-reviews
mailing list