[webkit-reviews] review denied: [Bug 58058] Replace WKStringGetCharactersPtr() with WKStringGetCharacters() : [Attachment 88659] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 7 10:40:43 PDT 2011
Adam Roben (:aroben) <aroben at apple.com> has denied Jeff Miller
<jeffm at apple.com>'s request for review:
Bug 58058: Replace WKStringGetCharactersPtr() with WKStringGetCharacters()
https://bugs.webkit.org/show_bug.cgi?id=58058
Attachment 88659: Patch
https://bugs.webkit.org/attachment.cgi?id=88659&action=review
------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88659&action=review
I should be encouraging you to write API tests for the new API you're adding.
They're pretty easy to write; just add some code to
Tools/TestWebKitAPI/Tests/WebKit2/WKString.cpp.
> Source/WebKit2/Shared/WebString.h:68
> + ASSERT(buffer);
> + if (!buffer)
> + return 0;
I don't think we're normally this defensive in the WK2 API.
> Source/WebKit2/Shared/WebString.h:72
> + size_t lengthInBytes = m_string.length() * sizeof(UChar);
> + if (bufferSize > lengthInBytes)
> + bufferSize = lengthInBytes;
> + memcpy(buffer, m_string.characters(), bufferSize);
This will cause a buffer overrun if bufferSize is less than lengthInBytes.
> Source/WebKit2/Shared/API/c/WKString.cpp:55
> +size_t WKStringGetCharacters(WKStringRef stringRef, WKChar* buffer, size_t
bufferSize)
It's surprising that this API takes a number bytes to copy rather than a number
of characters.
More information about the webkit-reviews
mailing list