Misc fixes #173
Misc fixes #173
Conversation
types.c
Outdated
| ucv_stringbuf_printf(pb, "%.14g", d); | ||
| else { | ||
| size_t len = ucv_stringbuf_printf(pb, "%.14g", d); | ||
| if (json && !strchr(pb->buf + pb->bpos - len, '.')) |
There was a problem hiding this comment.
Hmm, this feels slightly unelegant. What about this:
...
else if (json && trunc(d) == d)
ucv_stringbuf_printf(pb, "%.1f", d);
else
ucv_stringbuf_printf(pb, "%.14g", d);There was a problem hiding this comment.
I thought about doing it this way too, but I read in a few places that this can be prone to corner cases caused by weird floating point rounding issue. I think checking the output of sprintf is more reliable.
There was a problem hiding this comment.
can be prone to corner cases caused by weird floating point rounding issue
Hm, can you provide some links for this? My research so far seems to indicate that trunc(x) == x is indeed the way to go, but maybe I overlooked something.
The problem with the "scan backwards for a decimal point and append .0 if none found" approach is that it will fail if the %g format decides to yield an exponent form, e.g. we don't want to turn 1e100 into 1e100.0.
There was a problem hiding this comment.
Sorry, I can't find the original link anymore. I took a look at this from another angle and checked how existing JSON libraries are handling this issue. I looked at json-c, jansson and yajl, and they all use the scan-for-decimal-point approach. The code is rather different in each one, so I don't believe it was copied/cargo-culted between implementations. Would you accept my patch if I keep the approach, but fix the exponent form case?
There was a problem hiding this comment.
I pushed a fix based on your suggestion and including a testcase covering this corner case. Could you rebase this PR against master so that I can merge the remainder?
The `%g` printf format used for serializing double values into strings will not include any decimal place if the value happens to be integral, leading to an unwanted double to integer conversion when serializing and subsequently deserializing an integral double value as JSON. Solve this issue by checking the serialized string result for a decimal point or exponential notation and appending `.0` if neither is found. Ref: #173 Suggested-by: Felix Fietkau <[email protected]> Signed-off-by: Jo-Philipp Wich <[email protected]>
… macOS Signed-off-by: Felix Fietkau <[email protected]>
Accept char * const *, cast internally. Fixes a compile error Signed-off-by: Felix Fietkau <[email protected]>
|
Merged, thanks for your patience :) |
Two compile error fixes for macOS, and a fix for preserving the double type when generating JSON.