restore dtostrf when floats are disabled in printf/scanf + round fix#7087
restore dtostrf when floats are disabled in printf/scanf + round fix#7087d-a-v wants to merge 9 commits into
Conversation
d-a-v
changed the title
d-a-v
changed the title
|
d-a-v approved this pull request. |
| return utoa((unsigned int)value, result, base); | ||
| } | ||
|
|
||
| #ifdef NOPRINTFLOAT |
There was a problem hiding this comment.
NO_PRINTF_FLOAT?
NO_FLOAT_PRINTF?
There was a problem hiding this comment.
Dogpiling on this (sorry). Adding new defines normally ends up breaking PIO or other folks.
Would it be possible, through whatever magic the -u _printf_float uses, to somehow make the proper version be linked-in?
I could also drag this into the newlib if that would be needed to make it work "seamlessly" with the existing infra/flags.
There was a problem hiding this comment.
I thought of it as something like the existing NO_GLOBAL_... defines
PIO user btw, only change that would be required is adding env.Append(CXXFLAGS=["NO_PRINTF_FLOAT"]) to the user build script (or, modifying platformio.ini build_flags=... variable, which is also file that is controlled by user), Core files don't need to change
Re 'magic', it's just a simple != NULL:
https://github.com/earlephilhower/newlib-xtensa/blob/b326aa447a1e5b33d6ed7d164aa2c59eb3754b9f/newlib/libc/stdio/nano-vfprintf.c#L651-L653
There was a problem hiding this comment.
Oh, then could it be as simple as:
char * dtostrf(double number, signed char width, unsigned char prec, char *s) {
if (!_printf_float) {
return __dtostrf(number, width, char prec, s);
} else {
char fmt[32];
sprintf(fmt, "%%%d.%df", width, prec);
sprintf(s, fmt, number);
return s;
}
}
static char * __dtostrf(double number, signed char width, unsigned char prec, char *s) {
...all the @d-a-v fixed code here...
}
This way the existing flag which will be eval'd a compile time will either cause the dtostrf guts to be omitted and use printf, or vice-versa.
No flag changes, no tool flow changes. Assuming it works...
There was a problem hiding this comment.
great ! 👍 thanks @mcspr @earlephilhower
| #else // !NOPRINTFLOAT | ||
|
|
||
| char * dtostrf(double number, signed char width, unsigned char prec, char *s) { | ||
| char fmt[32]; |
There was a problem hiding this comment.
As I mentioned in the issue, does this need to have a safeguard such as asm(".global _printf_float");?
ref https://sourceware.org/binutils/docs/as/Global.html#Global
.global makes the symbol visible to ld.
Corrected stack start and end in stack_thunk_dump_stack().
allow versions 0.0.* to be special
earlephilhower
left a comment
earlephilhower
left a comment
There was a problem hiding this comment.
Thanks for un-borking my borkage! You went the extra mile and optimized for all cases, which is awesome.
Can we make it transparent, though? You're the weak-linkage expert on the team, I think. :)
There was a problem hiding this comment.
Dogpiling on this (sorry). Adding new defines normally ends up breaking PIO or other folks.
Would it be possible, through whatever magic the -u _printf_float uses, to somehow make the proper version be linked-in?
I could also drag this into the newlib if that would be needed to make it work "seamlessly" with the existing infra/flags.
…log limitation and other fixes (esp8266#6887) * upstream lwIP is now downloaded by a makefile, not subsubmoduled * lwip2: upstream lwIP not sub-sub-modules anymore lwip2: Allow IPv4 and IPv6 DNS and SNTP server configured via DHCP to co-exist (patch against upstream) * lwip2: enable tcp-listen-with-backlog feature * lwip2 submodule update: - enable more efficient chksum algorithm thanks to Richard Allen - enable tcp listener with backlog * more comments, fix backlog management, fix API * move default value definition in .cpp because one must not believe it can be redefined before including WiFiServer.h * improved backlog handling, it is no more a breaking change
* configTime(tzsec,dstsec,): fix UTC/local management This PR also remove dead code since probably newlib updates The NTP-TZ-DST example is also updated * restore sntp_set_timezone_in_seconds() fixes esp8266#6678 * +configTzTime()
6 participants
Replaced by #7093 (which updates only 1 file, not 23 :)
This PR restores
dtosrf()only when floats are disabled in printf.boards.txt builder adds a new define
-DNOPRINTFLOATin this case which enablesdtostrfcompilation.A small fix has been added for the rounding issue, which I believe is generic enough, but will always be a subject for discussion.
Some multiplications have been removed from the algorithm too.
fixes #7073 with solution 6
updates #7068
fixes #7043
dtostrf(1045.5, 0)now returns1046.@mcspr @arendst Can you test this ?