Re: [PATCH 3/3] introduce "format" date-mode

On Mon, Jul 20, 2015 at 08:41:08PM -0400, Eric Sunshine wrote:

quoted

Here's a patch, on top of jk/date-mode-format (I think it would also be
fine to just squash into the tip commit; the explanation in the commit
message is sufficiently mirrored in the code comment).

While cleaning up old local branches, I noticed that, although the
jk/date-mode-format topic[1] made it into 'next' (and will be merged
to 'master' according to "What's cooking"[2]), the below follow-on
patch[3] which improves strbuf_addftime() never got picked up. Was
this omission intentional? Based upon the discussion[4], I was under
the impression that the patch was considered reasonably acceptable
(and did not worsen problems with bogus format strings -- which are
bogus anyway).

Thanks for noticing. I do think the patch you quoted (to loop and grow
the strbuf) is a good change. The original code would easily bite
somebody with a really large date format, whereas this should work
sanely everywhere, short of malformed inputs. And even then, I'd expect
reasonable behavior on most systems. The obvious thing to worry about is
a system where feeding a malformed "% " causes strftime to return 0, no
matter what, and we reallocated and loop forever. But:

  1. I don't even know if such a system exists.

  2. We probably would blow up on malloc() eventually, so it wouldn't
     even be a "real" infinite loop.

So I think the worst case is probably that we get a report later on from
somebody on an arcane system that says "I fed crap to --date=format, and
my git died with an out-of-memory error", and then we figure out exactly
_how_ their system is weird and deal with it then.

-Peff