[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