Re: Bugs in strftime

[Rich Felker <dalias@libc.org>]

On Fri, Feb 02, 2018 at 06:07:38PM +0100, Dennis Wölfing wrote:
> Hi,
> 
> I recently wrote a test for strftime and ran it on multiple
> implementations. The source code of the test is available at [1]. The
> test results (also for other implementations) are available at [2].

Great! This has been a test deficiency we've had for a long time and
was a big part of what kept me from reviewing and merging recent
patches to strftime in a timely manner.

> On musl my test currently reports 8 failures. These are caused by two bugs:
> 
> 1. For the + flag, musl does currently only prefix the output by a plus
> when the number without padding consumes more the 4 bytes (2 for %C).
> However according to the POSIX standard, there should be a leading plus
> when "the field being produced consumes more than four bytes" (2 for
> %C). The padding is part of the field being produced.

I've actually discussed this before, being doubtful about whether the
current behavior was correct, but was unable to find any authoritative
interpretation. Do you know if there is one?

> So for example %+3C should always have a leading plus for any
> non-negative years because the field then always has a width of at least
> 3 bytes. (There is also an example in the POSIX standard where "%+5Y"
> produces "+0270" for the year 270.)

While rationale is not itself authoritative, it looks like it contains
enough information to differentiate the intent of the ambiguous text.
Thanks!

> This bug is causing these failures:
> > "%+3C": expected "+20", got "020"
> > "%+11F": expected "+2016-01-03", got "02016-01-03"
> > "%+5G": expected "+2015", got "02015"
> > "%+5Y": expected "+2016", got "02016"
> > "%+5Y": expected "+0000", got "00000"

I'll need to look over how to change the logic to match the desired
behavior but it shouldn't be hard.

> 2. %F produces incorrect results for the year 0 when a field width is
> specified. It seems like in this case strftime does not output the year
> and applies the padding to the month.

Ah, it's the code in the top-level function that strips leading sign
and zeros to do the padding:

	for (; *t=='+' || *t=='-' || (*t=='0'&&t[1]); t++, k--);

I think instead if should do something like:

	if (*t=='+' || *t=='-') t++, k--;
	for (; (*t=='0'&&t[1]); t++, k--);

In other words, only strip + or - from the first character, not later
in the string.

> This bug is causing these failures:
> > "%01F": expected "0-02-23", got "2-23"
> > "%06F": expected "0-02-23", got "002-23"
> > "%010F": expected "0000-02-23", got "0000002-23"
> 
> The tests were run on musl 1.1.18 on Alpine Linux.
> 
> [1] https://github.com/dennis95/dennix/blob/master/libc/test/test-strftime.c
> 
> [2]
> https://gist.github.com/dennis95/b4869b5cbb3c21e15e409afb827354a5#file-musl-1-1-18-alpine-linux-x86_64

Thanks again for doing this testing and reporting it. Would you be
interested in helping get these tests into our libc-test package?

Rich