Bug 201020 – Rename StringBuilder functions to avoid unclear "append uninitialized" terminology

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Darin Adler]

Rename StringBuilder functions to avoid unclear "append uninitialized" terminology

[Darin Adler]

Created attachment 376964 [details]
Patch

[Darin Adler]

Created attachment 376972 [details]
Patch

[Alex Christensen]

Comment on attachment 376972 [details]
Patch

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

> Source/WTF/wtf/text/StringBuilder.cpp:134
> +    std::memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(UChar)); // This can't overflow.

Why did you remove the other "This can't overflow." comment but not this one?

[Darin Adler]

Comment on attachment 376972 [details]
Patch

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

>> Source/WTF/wtf/text/StringBuilder.cpp:134
>> +    std::memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(UChar)); // This can't overflow.
> 
> Why did you remove the other "This can't overflow." comment but not this one?

The comment here has a meaning even though it's not as specific and clear as I would like: limits enforced on m_length elsewhere in this class and other WTF::String family classes are presumably the reason this multiplication by 2 can’t overflow, even on a 32-bit platform where size_t is 32-bit.

The other comment was different. It was multiplying by sizeof(LChar), which is 1; obviously multiplying by 1 won’t overflow. I removed both the multiplication and the comment because I don’t think we need an explicit typecast and multiplication; then there’s no math at all so no need to say anything about overflow. If we really were worried that sizeof(LChar) == 1 was non-obvious we could have put in a compile time assertion, but I didn’t think that was needed either.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 376972 [details]:

webgl/2.0.0/conformance/ogles/GL/mat/mat_025_to_032.html bug 201043 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 376972 [details]
Patch

Clearing flags on attachment: 376972

Committed r249015: <https://trac.webkit.org/changeset/249015>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54602473>