[webkit-reviews] review denied: [Bug 72793] Add flags/precision arguments to String::number(double) to allow fine-grained control over the result string : [Attachment 115939] Patch

[bugzilla-daemon at webkit.org]

Zoltan Herczeg <zherczeg at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 72793: Add flags/precision arguments to String::number(double) to allow
fine-grained control over the result string
https://bugs.webkit.org/show_bug.cgi?id=72793

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

------- Additional Comments from Zoltan Herczeg <zherczeg at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115939&action=review


> Source/JavaScriptCore/runtime/NumberPrototype.cpp:399
> -    DoubleConversionStringBuilder builder(buffer,
WTF::NumberToStringBufferLength);
> -    const DoubleToStringConverter& converter =
DoubleToStringConverter::EcmaScriptConverter();
> -    builder.Reset();
> -    converter.ToFixed(x, decimalPlaces, &builder);
> -    return JSValue::encode(jsString(exec, UString(builder.Finalize())));
> +    return JSValue::encode(jsString(exec,
UString(numberToFixedWidthString(x, decimalPlaces, buffer))));

I think this code now is unsafe. The previous code destroyed "builder" after
the return statement, the new code destroys it inside the utility function so
accessing the buffer later is unsafe.