[webkit-reviews] review granted: [Bug 56099] Introduce WTF HexNumber.h : [Attachment 86045] Patch v6

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 56099: Introduce WTF HexNumber.h
https://bugs.webkit.org/show_bug.cgi?id=56099

Attachment 86045: Patch v6
https://bugs.webkit.org/attachment.cgi?id=86045&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=86045&action=review

> Source/JavaScriptCore/wtf/HexNumber.h:53
> +// Special variant for CSSParser, that eventually appends just a single hex
digit, if the byte fits.
> +template<typename T>
> +inline void placeByteAsHex(unsigned char byte, T& destination, unsigned&
index, HexConversionMode mode = Uppercase)

This special variant needs a different name.

> Source/JavaScriptCore/wtf/HexNumber.h:72
> +// If desiredDigits=-1: it will create as many hex digits as needed to
represent the number.
> +// Otherwhise, it uses exactly 'desiredDigits' for the conversion, use with
caution.
> +template<typename T>
> +inline void appendUnsignedAsHex(unsigned number, T& destination, int
desiredDigits = -1, HexConversionMode mode = Uppercase)

I’d rather see this as two separate functions. The magic number -1 seems
undesirable.

> Source/WebCore/inspector/InspectorResourceAgent.cpp:461
> -    static const char hexDigits[17] = "0123456789ABCDEF";
> -    size_t bufferSize = length * 3 - 1;
> -    StringBuffer buffer(bufferSize);
> -    size_t index = 0;
> +    Vector<UChar> buffer;
> +    buffer.reserveInitialCapacity(length * 3 - 1);

Gavin says StringBuilder is better than Vector<UChar> now, so lets use
StringBuilder here.