[webkit-reviews] review denied: [Bug 88936] StringImpl::characters can return NULL for an empty string : [Attachment 147440] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 13 18:34:29 PDT 2012
Darin Adler <darin at apple.com> has denied Myles C. Maxfield
<mmaxfield at google.com>'s request for review:
Bug 88936: StringImpl::characters can return NULL for an empty string
https://bugs.webkit.org/show_bug.cgi?id=88936
Attachment 147440: Patch
https://bugs.webkit.org/attachment.cgi?id=147440&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=147440&action=review
> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:99
> + static UChar dummy;
No need to make this static. A pointer to the stack is just as good as a
pointer to a global variable.
> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:100
> + if (!lhs && !lhsLength)
We don’t need to check both of these. Doesn’t matter which we check, but I
don’t want an extra branch to check both. My preference would be to only check
the length.
> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:101
> + lhs = &dummy;
WebKit project indents 4 spaces, not 2.
> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:103
> + if (!rhs && !rhsLength)
> + rhs = &dummy;
Same comments again.
More information about the webkit-reviews
mailing list