make RoundTripDoubleToBuffer locale-independent#2082
Conversation
derekmauro
left a comment
derekmauro
left a comment
There was a problem hiding this comment.
This is a good catch, but I have a few comments. Let me know if you want me to make the suggested improvements, or if you would like to handle them.
| // round-trippable through SimpleAtod(). | ||
| const char radix = *localeconv()->decimal_point; | ||
| if (radix != '.') { | ||
| if (char* p = strchr(buffer, radix)) *p = '.'; |
There was a problem hiding this comment.
If localeconv()->decimal_point is empty (which might happen in some minimal environments), radix will be \0. In this case, strchr(buffer, '\0') will find the NUL terminator and replace it with ., corrupting the string.
I also wonder if there is any chance that localeconv()->decimal_point will be multibyte.
| EXPECT_TRUE( | ||
| absl::SimpleAtod(absl::StrCat(absl::HighPrecision(0.1)), &parsed)); | ||
| EXPECT_EQ(parsed, 0.1); | ||
| setlocale(LC_NUMERIC, old_locale.c_str()); |
There was a problem hiding this comment.
It would be better to use absl::Cleanup as an RAII to restore this around line 1728.
| snprintf_result < numbers_internal::kFastToBufferSize); | ||
| } | ||
|
|
||
| // snprintf() writes the radix character chosen by the global C locale's |
There was a problem hiding this comment.
In theory we should be able to use std::to_chars for locale independence, but compilers were late to implement full floating-point support, and we still support some of these older compilers. Maybe a TODO would be nice here.
2 participants
RoundTripDoubleToBuffer formats with snprintf %g, whose radix character follows the global C locale's LC_NUMERIC category, so in a process that has called setlocale() to something like de_DE or fr_FR absl::HighPrecision(d) comes out as e.g. "0,1". SimpleAtod only accepts '.', so the value no longer parses back to itself even though round-tripping through SimpleAtod is exactly what HighPrecision promises, and the rest of the float formatting here (RoundTripFloatToBuffer, SixDigitsToBuffer) is already locale-independent. This rewrites the radix character back to '.' in the produced buffer, and adds a regression test that exercises HighPrecision under a comma-radix locale (skipped when no such locale is installed).