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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2018, 17:52 UTC
Message-ID: <xmqqy3iouf4b.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <66189602d022dd1911b5f5d98594506b80bb81ff.1521478106.git.johannes.schindelin@gmx.de>

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

Show 8 quoted lines

> 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?