Skip to content

unittests: Fix printf float test BUFSIZE#8243

Merged
smlng merged 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/tests-snprintf
Dec 19, 2017
Merged

unittests: Fix printf float test BUFSIZE#8243
smlng merged 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/tests-snprintf

Conversation

@jnohlgard
Copy link
Copy Markdown
Member

@jnohlgard jnohlgard commented Dec 12, 2017

Contribution description

Fixes the following error from GCC 7.2.0 with a recent newlib

In file included from /usr/arm-none-eabi/include/stdio.h:800:0,
                 from /home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:21:
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c: In function ‘sfprintf_float’:
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:39:28: error: ‘%f’ directive output truncated writing 11 bytes into a region of size 10 [-Werror=format-truncation=]
     snprintf(str, BUFSIZE, "%f", in0);
                            ^
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:39:5: note: ‘__builtin_snprintf’ output 12 bytes into a destination of size 10
     snprintf(str, BUFSIZE, "%f", in0);
     ^
cc1: all warnings being treated as errors

The expected test output had to be updated as well, so it might be a good idea to re-run this on some other newlib versions. Tests pass on native and mulle.

Issues/PRs references

#8265

Fixes the following error from GCC 7.2.0 with a recent newlib

In file included from /usr/arm-none-eabi/include/stdio.h:800:0,
                 from /home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:21:
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c: In function ‘sfprintf_float’:
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:39:28: error: ‘%f’ directive output truncated writing 11 bytes into a region of size 10 [-Werror=format-truncation=]
     snprintf(str, BUFSIZE, "%f", in0);
                            ^
/home/jgn/work/src/riot/tests/unittests/tests-printf_float/tests-printf_float.c:39:5: note: ‘__builtin_snprintf’ output 12 bytes into a destination of size 10
     snprintf(str, BUFSIZE, "%f", in0);
     ^
cc1: all warnings being treated as errors
@jnohlgard jnohlgard added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tests Area: tests and testing framework labels Dec 12, 2017
@jnohlgard jnohlgard added this to the Release 2018.01 milestone Dec 12, 2017
@jnohlgard jnohlgard self-assigned this Dec 12, 2017
@jnohlgard jnohlgard mentioned this pull request Dec 14, 2017
13 tasks
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, tested on macOS, too. No issue with this changes!

@jnohlgard
Copy link
Copy Markdown
Member Author

I tested with a build made with the current Docker riot/riotbuild image and it still works on Mulle (GCC 6.x). I think this is safe to merge. The C standard specifies that the %f printf format specifier when used without precision should give 6 digits after the decimal point.

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 19, 2017

without precision should give 6 digits after the decimal point.

well should is not MUST in IETF RFC speech 😉 So, maybe it's safe to explicitly force 6 digits by specifying that?

@jnohlgard
Copy link
Copy Markdown
Member Author

jnohlgard commented Dec 19, 2017

@smlng let's keep this until we find a C library which does not comply with the 6 digits by default rule.

Edit: Do you agree?

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 19, 2017

ACK & GO

@smlng smlng merged commit b151f95 into RIOT-OS:master Dec 19, 2017
@jnohlgard jnohlgard deleted the pr/tests-snprintf branch December 19, 2017 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants