On Fri, Mar 28, 2014 at 12:02:46PM -0700, Junio C Hamano wrote: > >> - 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 > ;-) I'm not sure I understand. Even if we know N, we've lost information during the truncation done by time_t (we cannot distingiuish true M from N+M). > > 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. Do you mean "let's not worry about the absurdly specific overflow case", or "let's not do this gross time_to_tm hack?" This (non-)issue has consumed a lot more brain power than it is probably worth. I'd like to figure out which patch to go with and be done. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html