Re: [PATCH] mingw: abort on invalid strftime formats

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> On Windows, strftime() does not silently ignore invalid formats, but
> warns about them and then returns 0 and sets errno to EINVAL.
>
> Unfortunately, Git does not expect such a behavior, as it disagrees
> with strftime()'s semantics on Linux. As a consequence, Git
> misinterprets the return value 0 as "I need more space" and grows the
> buffer. As the larger buffer does not fix the format, the buffer grows
> and grows and grows until we are out of memory and abort.

Yuck, it is bad that the callers assume 0 is always "need more
space", as "If a conversion specification does not correspond to any
of the above, the behavior is undefined." is what we get from POSIX.

> So let's just bite the bullet and override strftime() for MINGW and
> abort on an invalid format string. While this does not provide the
> best user experience, it is the best we can do.

As long as we allow arbitrary end-user input passed to strftime(),
this is probably a step in the right direction.  

As to the future direction, I however wonder if we can return an
error from here and make the caller cooperate a bit more.  Of
course, implementation of strftime() that silently ignore invalid
formats would not be able to return correct errors like an updated
mingw_strftime() that does not die but communicates error to its
caller would, though.

My "git grep" is hitting only one caller of strftime(), which is
strbuf_addftime(), which already does some magic to the format
string, so such a future enhancement may not be _so_ bad.

Will apply, thanks.  I do not think there is no reason to cook this
in 'next', and would assume this can instead go directly to 'master'
to be part of v2.17-rc1 and onward, right?