/usr/bin/printf (so also internal printf in sh) ignores width and precision in %b format. The change is since commit base r265706 (May 2014), where macro PF() with width and precision interpretation has been replaced by direct call to fputs(), so width and precision are ignored now. Expected result, for example from bash: bash$ printf "%8.2b" "a\nb\n" a Bad result from /usr/bin/printf and from sh: sh$ /usr/bin/printf "%8.2b" "a\nb\n" a b sh$ printf "%8.2b" "a\nb\n" a b
Created attachment 195011 [details] Possible fix Fix submitted by Garrett D'Amore.
Created attachment 195012 [details] Possible fix + copyright update
This looks like a valid bug. This change should have some tests. The printf utility already has tests so adding tests for this bug seems appropriate. The copy of the format string is unnecessary as `start` is already a copy. Getting rid of it would also fix the bug that there is a memory leak if the allocation for bfmt succeeds but the allocation for p fails. (On another note, `start` can be a rather large allocation on the stack. Ideally, no space would be reserved past the specifier such as b, c, s.)
Created attachment 195131 [details] Take 2 Something like this (Not sure about the test).
(In reply to Pedro F. Giffuni from comment #4) I have no idea what fails when I add the test: ___ kyua test -k /usr/tests/usr.bin/sed/Kyuafile inplace_race_test:main -> passed [0.222s] legacy_test:main -> passed [0.335s] multi_test:main -> failed: 3 of 130 tests failed [1.086s] sed2_test:inplace_hardlink_src -> passed [0.041s] sed2_test:inplace_symlink_src -> passed [0.022s] sed_test:c2048 -> passed [0.020s] sed_test:emptybackref -> passed [0.025s] sed_test:longlines -> passed [0.026s] sed_test:preserve_leading_ws_ia -> passed [0.019s] sed_test:rangeselection -> passed [0.089s] Results file id is usr_tests_usr.bin_sed.20180730-183642-566700 Results saved to /home/pfg/.kyua/store/results.usr_tests_usr.bin_sed.20180730-183642-566700.db ___ I just don't know my way around the testsuite. I am tempted to commit the fix and let someone else deal with the test.
For reference here is the replicated bug on Illumos: https://www.illumos.org/issues/9646
A commit references this bug: Author: pfg Date: Tue Aug 7 23:03:51 UTC 2018 New revision: 337440 URL: https://svnweb.freebsd.org/changeset/base/337440 Log: Fix printf(1) ignores width and precision in %b format. The precision with behavior is "unspecified" by POSIX (as of 2018), but most implementations seem to have taken it to be treated the same as for "s"; applied after the unescaping. Adopt the same treatment on our printf. PR: 229641 Submitted by: Garrett D'Amore (illumos) Changes: head/usr.bin/printf/printf.c
(In reply to Pedro F. Giffuni from comment #5) While I'd love to have an ATF test, and I will continue playing with the testsuite at a later time, ... it does seem this is undefined behavior, so a test is not really mandatory (?). For now, it does seem a good idea to "fix" the issue before the next release (12.0) so I commited the fix.
A commit references this bug: Author: pfg Date: Wed Aug 8 15:25:01 UTC 2018 New revision: 337458 URL: https://svnweb.freebsd.org/changeset/base/337458 Log: Fix printf(1) ignores width and precision in %b format. The precision with the conversion specifier b is specified by POSIX: see point 7 in the reference documentation. This corrects previous wrong log in r337440. Reference: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html PR: 229641 Reported by: Rudolf Cejka Submitted by: Garrett D'Amore (illumos) MFC after: 1 week Changes: head/usr.bin/printf/printf.c
(In reply to Pedro F. Giffuni from comment #8) Now that its clear the behaviour is specified by POSIX (Thanks Rudolf for pointing it out), the tests are more important, as is MFCing the change. Help with the test/ATF is welcome :-/.
A commit references this bug: Author: jilles Date: Sat Aug 11 11:13:35 UTC 2018 New revision: 337618 URL: https://svnweb.freebsd.org/changeset/base/337618 Log: printf: Fix \c in %b in printf builtin exiting the shell after r337458 SVN r337458 erroneously partially reverted r265885. This is immediately visible when running the Kyua/ATF tests for usr.bin/printf, which actually test sh's printf builtin. PR: 229641 Changes: head/usr.bin/printf/printf.c
With the fix from SVN r337618 and svn ps fbsd:nokeywords y usr.bin/printf/tests/regress.bwidth.out, the tests from the patch "Take 2" pass and look good to me.
A commit references this bug: Author: jilles Date: Mon Aug 13 21:54:28 UTC 2018 New revision: 337728 URL: https://svnweb.freebsd.org/changeset/base/337728 Log: printf: Add test for width and precision in %b format PR: 229641 Submitted by: pfg Changes: head/usr.bin/printf/tests/Makefile head/usr.bin/printf/tests/regress.bwidth.out head/usr.bin/printf/tests/regress.sh
A commit references this bug: Author: pfg Date: Tue Aug 14 22:57:29 UTC 2018 New revision: 337823 URL: https://svnweb.freebsd.org/changeset/base/337823 Log: MFC r337458, r337618: Fix printf(1) ignores width and precision in %b format. The precision with the conversion specifier b is specified by POSIX: see point 7 in the reference documentation. Include fix from jilles@ to avoid breaking the testsuite. Reference: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html PR: 229641 Reported by: Rudolf Cejka Submitted by: Garrett D'Amore (illumos) Changes: _U stable/11/ stable/11/usr.bin/printf/printf.c
A commit references this bug: Author: pfg Date: Wed Aug 22 04:27:34 UTC 2018 New revision: 338181 URL: https://svnweb.freebsd.org/changeset/base/338181 Log: MFC r337728: (committed by jilles) printf: Add test for width and precision in %b format PR: 229641 Changes: _U stable/11/ stable/11/usr.bin/printf/tests/Makefile stable/11/usr.bin/printf/tests/regress.bwidth.out stable/11/usr.bin/printf/tests/regress.sh
Committed in 12 and 11 (doesn't apply cleanly in 10 and I can't test at this time). Thanks!