Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 28, 2014, 19:02 UTC
- Message-ID
- <xmqqppl69meh.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20140328184710.GA29987@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> On Fri, Mar 28, 2014 at 09:41:53AM -0700, Junio C Hamano wrote: > >> Offhand, the three possible failure modes this thread identified >> sounds to me like the only plausible ones, and I think the best way >> forward might be to >> >> - teach the "is the result sane, even though we may have got a >> non-NULL from gmtime? otherwise let's signal a failure by >> replacing it with a known sentinel value" codepath the new >> failure mode Charles's report suggests---if we feed a positive >> timestamp and gmtime gave us back a tm_year+1900 < 0, that is >> certainly an overflow; and > > I don't think we can analyze the output from gmtime. If it wraps the > year at N, then won't N+2014 look like a valid value?
Yes, but I was hoping that there are small number of possible N's ;-)
Show 22 quoted lines
> If we are going to do something trustworthy I think it has to be before
> we hand off to gmtime. Like:
>
> diff --git a/date.c b/date.c
> index e1a2cee..e0c43c4 100644
> --- a/date.c
> +++ b/date.c
> @@ -57,6 +57,8 @@ static time_t gm_time_t(unsigned long time, int tz)
> static struct tm *time_to_tm(unsigned long time, int tz)
> {
> time_t t = gm_time_t(time, tz);
> + if (t > 9999999999999999)
> + return NULL;
> return gmtime(&t);
> }
>
> I suspect that would handle the FreeBSD case, as well.
>
> By the way, I have a suspicion that the gm_time_t above can overflow if
> you specially craft a value at the edge of what time_t can handle (we
> check that our value will not overflow time_t earlier, but now we might
> be adding up to 86400 seconds to it). <sigh>Yuck. Let's not go there.
Thanks.