Re: [PATCH 4/5] log: handle integer overflow in timestamps

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 24, 2014 at 11:50:00AM -0800, Junio C Hamano wrote:
>
>> > We also recognize overflow in the timezone field, which
>> > could produce nonsensical results. In this case we show the
>> > parsed date, but in UTC.
>> 
>> Both are good measures to fallback to sanity, but why is that
>> if/else?  In other words...
>> 
>> > +	if (date_overflows(date))
>> > +		date = 0;
>> > +	else {
>> > +		if (ident->tz_begin && ident->tz_end)
>> > +			tz = strtol(ident->tz_begin, NULL, 10);
>> > +		if (tz == LONG_MAX || tz == LONG_MIN)
>> > +			tz = 0;
>> > +	}
>> 
>> ... don't we want to fix an input having a bogus timestamp and also
>> a bogus tz recorded in it?
>
> If there is a bogus timestamp, then we do not want to look at tz at all.
> We leave it at "0", so that we get a true sentinel:

Ah, OK, I missed the initialization to 0 at the beginning.

It might have been more clear if "int tz" declaration were left
uninitialized, and the variable were explicitly cleared to 0 in the
"date-overflows" error codepath, but it is not a big deal.

Thanks for clarification.