From: Junio C Hamano Date: Mon, 24 Feb 2014 20:21:33 GMT Subject: Re: [PATCH 4/5] log: handle integer overflow in timestamps Message-ID: In-Reply-To: <20140224195829.GA11940@sigill.intra.peff.net> Jeff King 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.