[Webkit-unassigned] [Bug 17017] Remove KJS::UChar

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 26 08:18:29 PST 2008


http://bugs.webkit.org/show_bug.cgi?id=17017


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18705|review?                     |review-
               Flag|                            |




------- Comment #6 from darin at apple.com  2008-01-26 08:18 PDT -------
(From update of attachment 18705)
Generally looks good.

+    return toRef(UString(reinterpret_cast<const UChar*>(chars),
static_cast<int>(numChars)).rep()->ref());

+    return toRef(UString(reinterpret_cast<UChar*>(buffer.data()), p -
buffer.data()).rep()->ref());

+  const ::UChar* d = reinterpret_cast<const ::UChar*>(&data()[0]);

+    Completion comp = Interpreter::evaluate(exec, filename, baseLine,
reinterpret_cast<const UChar*>(str.characters()), str.length(), thisNode);

+    return Identifier(reinterpret_cast<const UChar*>(m_impl->characters()),
m_impl->length());

+    return UString(reinterpret_cast<const UChar*>(m_impl->characters()),
m_impl->length());

No typecast needed at all at these call sites now.

There's a small but potentially slightly dangerous difference between
KJS::UChar and the integer type UChar typedef, and that's the behavior of
converting a char into UChar. This will silently behave differently, sign
extending rather than padding with zeroes. Here are the three callsites that
are affected:

>From CStringTranslator::translate:

        for (size_t i = 0; i != length; i++)
            d[i] = c[i];

This needs a typecast to "unsigned char" on the right side of the assignment
statement. Unless we guarantee we only use this on ASCII strings.

>From SourceStream& SourceStream::operator<<(char c):

    UChar ch(c);

This also needs a typecast to "unsigned char". It's probably a little less
dangerous than the above because we probably use this operator only with ASCII
characters, but I'm not sure.

>From UString::append(const char*), in 3 different places:

        for (int i = 0; i < tSize; ++i)
            d[thisSize + i] = t[i];

Same issue, same explanation -- safe if only used on ASCII, or we can add a
cast to preserve the old behavior.

I'm going to say review- because of this issue; otherwise good patch.


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



More information about the webkit-unassigned mailing list