[Webkit-unassigned] [Bug 69913] Use realloc() to expand/shrink StringBuilder buffer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69913





--- Comment #5 from Xianzhu Wang <wangxianzhu at chromium.org>  2011-10-12 19:06:00 PST ---
(From update of attachment 110654)
View in context: https://bugs.webkit.org/attachment.cgi?id=110654&action=review

Thanks Darin for review. I shouldn't have been that hurry going home without running check-webkit-style and checking carefully :)

>> Source/JavaScriptCore/wtf/text/StringBuilder.cpp:110
>> +    // otherwise fallback to "allocate and copy" method.
> 
> The verb is “fall back”. The word “fallback” is a noun. This should say “fall back”.

Done.

>> Source/JavaScriptCore/wtf/text/StringImpl.cpp:93
>> +PassRefPtr<StringImpl> StringImpl::reallocate(PassRefPtr<StringImpl> impl, unsigned length, UChar*& data)
> 
> I think “impl” is a strange name for this argument. You could just as easily call it “string”. I’d probably call it oldString or existingString or originalString.

Changed to originalString.

>> Source/JavaScriptCore/wtf/text/StringImpl.cpp:112
>> +    return result;
> 
> The style bot is right to complain about the use of PassRefPtr. The best way to fix it here is to just call it right in the return statement.

Done. This was caused by incomplete debug code cleanup.

>>> Source/JavaScriptCore/wtf/text/StringImpl.h:187
>>> +    static PassRefPtr<StringImpl> reallocate(PassRefPtr<StringImpl> impl, unsigned length, UChar*& data);
>> 
>> The parameter name "impl" adds no information, so it should be removed.  [readability/parameter_name] [5]
> 
> I agree with the style bot on this.

Changed to originalString and use that name in the comments.

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