Bug 50123 - Add an overload to makeString for Vector<char>
Summary: Add an overload to makeString for Vector<char>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-27 09:14 PST by Patrick R. Gansterer
Modified: 2010-12-11 04:49 PST (History)
4 users (show)

See Also:


Attachments
Patch (6.47 KB, patch)
2010-11-27 12:06 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff
Patch for landing (7.31 KB, patch)
2010-12-11 03:29 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-11-27 09:14:26 PST
see patch
Comment 1 Patrick R. Gansterer 2010-11-27 12:06:01 PST
Created attachment 74963 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-12-09 23:53:26 PST
Darin or or Maciej are better reviewers here.
Comment 3 Darin Adler 2010-12-10 10:40:41 PST
Comment on attachment 74963 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74963&action=review

> JavaScriptCore/wtf/text/StringConcatenate.h:124
> +    unsigned length() { return m_buffer.size(); }
> +
> +    void writeTo(UChar* destination)
> +    {
> +        for (unsigned i = 0; i < m_buffer.size(); ++i)
> +            destination[i] = m_buffer[i];

It’s better style to use size_t, which makes the type of vector’s size than unsigned, which can be narrower.

This will widen characters to UChar based on the signed-ness of characters on a particular platform. If the characters are all ASCII, that’s OK, but if there are non-ASCII characters we probably want the characters treated as Latin-1 as we do in most other contexts. To accomplish that, we will need to cast the characters to unsigned char before letting them get widened to UChar.

The same applies for const char*.

We should either have an isASCII assertion or a typecast to unsigned char.
Comment 4 Patrick R. Gansterer 2010-12-11 03:29:00 PST
Created attachment 76299 [details]
Patch for landing
Comment 5 WebKit Review Bot 2010-12-11 04:49:07 PST
Comment on attachment 76299 [details]
Patch for landing

Clearing flags on attachment: 76299

Committed r73853: <http://trac.webkit.org/changeset/73853>
Comment 6 WebKit Review Bot 2010-12-11 04:49:13 PST
All reviewed patches have been landed.  Closing bug.