[webkit-reviews] review granted: [Bug 37949] Do no copy strings into a shared buffer when converting UStrings to Strings : [Attachment 53989] The patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 21 14:12:48 PDT 2010
Darin Adler <darin at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 37949: Do no copy strings into a shared buffer when converting UStrings to
Strings
https://bugs.webkit.org/show_bug.cgi?id=37949
Attachment 53989: The patch
https://bugs.webkit.org/attachment.cgi?id=53989&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> String identifierToString(const JSC::Identifier&);
> String ustringToString(const JSC::UString&);
> JSC::UString stringToUString(const String&);
> -
> AtomicString identifierToAtomicString(const JSC::Identifier&);
> AtomicString ustringToAtomicString(const JSC::UString&);
> AtomicStringImpl* findAtomicString(const JSC::Identifier&);
I think that blank line is good and you need not remove it.
> + inline String ustringToString(const JSC::UString& u)
> + {
> + return u.rep();
> + }
> +
> + inline JSC::UString stringToUString(const String& s)
> + {
> + return JSC::UString(s.impl());
> + }
> +
> + inline String identifierToString(const JSC::Identifier& i)
> + {
> + return i.ustring().rep();
> + }
> +
> + inline AtomicString ustringToAtomicString(const JSC::UString& u)
> + {
> + return AtomicString(u.rep());
> + }
> +
> + inline AtomicString identifierToAtomicString(const JSC::Identifier&
identifier)
> + {
> + return AtomicString(identifier.ustring().rep());
> + }
> +
> String valueToStringWithNullCheck(JSC::ExecState*, JSC::JSValue); //
null if the value is null
> String valueToStringWithUndefinedOrNullCheck(JSC::ExecState*,
JSC::JSValue); // null if the value is null or undefined
I suggest putting the function bodies at the end of the file. No need for these
definitions to be close the the declarations.
r=me with or without those changes
More information about the webkit-reviews
mailing list