-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
check boundaries converting float/double to String #613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Memory usage change @ db1b6a0
Click for full report table
Click for full report CSV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we are still off by one because FLT_MAX_10_EXP is the exponent value of the integer part represented in base 10, this means that can be zero an still represent a valid digit.
A more correct implementation can be:
(FLT_MAX_10_EXP + 1) + FLT_MAX_DECIMAL_PLACES + 1 /* '-' */ + 1 /* '.' */ + 1 /* '\0' */
Please note that the bug is also present in the original commit in the library, because was wrongly ported from the original fix in samd core that was correct.
| typedef void (String::*StringIfHelperType)() const; | ||
| void StringIfHelper() const {} | ||
|
|
||
| static size_t const FLT_MAX_DECIMAL_PLACES = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to propose to use FLT_DECIMAL_DIG and DBL_DECIMAL_DIG instead of harding both to 10 that is somewhere in the middle of the real possible values (7 and 16). Or if we wanto to stay C99 DECIMAL_DIG.
Backport of arduino/ArduinoCore-API@952d776