[webkit-reviews] review denied: [Bug 96132] Avoid strlen() when converting DTOA result to String : [Attachment 162814] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 96132: Avoid strlen() when converting DTOA result to String
https://bugs.webkit.org/show_bug.cgi?id=96132

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

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


review- because patch doesn’t build and so needs some additional work; probably
should review a version that does build

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:388
> +    WTF::NumberToStringBuffer buffer;

The WTF:: prefix should not be needed here.

> Source/WTF/wtf/dtoa.h:47
> -WTF_EXPORT_PRIVATE const char* numberToString(double, NumberToStringBuffer);

> -WTF_EXPORT_PRIVATE const char* numberToFixedPrecisionString(double, unsigned
significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> -WTF_EXPORT_PRIVATE const char* numberToFixedWidthString(double, unsigned
decimalPlaces, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToString(double, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToFixedPrecisionString(double, unsigned
significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> +WTF_EXPORT_PRIVATE int numberToFixedWidthString(double, unsigned
decimalPlaces, NumberToStringBuffer);

This return value is a lot less self-explanatory than the old one. We probably
need a comment or some other form of explanation. Perhaps this should even be
an out argument instead of a return value for clarity.