Bug fix: decimalPlaces will not return correctly for a large double value

[Binyang2014]

When use stringWriterBuilder to write a really large double value, the program may return an incorrect value if we set the precisionType to decimalPlaces. The root cause for this issue is we not check if we convert double to string successfully (sometime the is buffer will be too small to contain the whole string). For this case, I will use significantDigits type instead, and set the precision to 17.

[cdunn2001]

That's a great catch! However, shouldn't we also increase the buffer size so the if-antecedent is never true? In fact, if we make the buffer is big enough, can we turn the consequent into an exception for the case which should never occur?

[Binyang2014]

Thanks Christopher, but I think we can't know the buffer size until we call snprintf function. I believe there is no max length for the double value which use fixed decimal. So if we want to make the buffer big enough, we need to alloc the buffer dynamically. In this way, we need to call snprintf first to get the correct buffer size, then alloc the buffer.

Since we already set the max precision for double value to 17. For a fixed decimal value, if it exceed the buffer size, I think we can use significant decimal type instead.

[BillyGoto]

On Thu, May 10, 2018 at 2:06 AM Binyang2014 ***@***.***> wrote: Thanks Christopher, but I think we can't know the buffer size until we call snprintf function. I believe there is no max length for the double value which use fixed decimal.

Max of a 64-bit double is like 10^308 or something, so there’s a biggish but not crazy limit of decimal places. So if we want to make the buffer big enough, we need to alloc the buffer

dynamically. In this way, we need to call snprintf first to get the correct buffer size, then alloc the buffer. Since we already set the max precision for double value to 17. For a fixed decimal value, if it exceed the buffer size, I think we can use significant decimal type instead. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHFzPDMep0VsHyZLNxwS-BW0uxQImmjwks5tw9jtgaJpZM4T4Tud> .

-- ǝnɥɐuop ʎllıq

[BillyDonahue]

This assert happens way too late. We already used len as a range limit above in the calls to fixNumericLocale etc.
Also the "%%.%ug" 2-stage snprintf is wasteful. We can just use .* precision.

I would like to refactor this function before changing it. I'll send a PR.

[Binyang2014]

Thanks Billy, I will wait for your code refactor.