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

From: Jeff King <peff@peff.net>
Date: Jul 21, 2015, 01:19 UTC
Message-ID: <20150721011931.GB28255@peff.net>
In-Reply-To: <CAPig+cT_3pFH6XKBO1F2rbNyiGz5NgLMMiA_T+NWBroW5Sj7cg@mail.gmail.com>

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

Show 12 quoted lines

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