Re: [PATCH 1/1] Fix date checking in case if time was not initialized.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2013, 22:07 UTC
Message-ID: <7v4nh0oxnv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAHXAxrO4c=s0pjNpXK171HUbQT06jm-VAxNNK1DAqZEZfz6OtA@mail.gmail.com>

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

Show 8 quoted lines

>>> So is_date() always return negative result for the text string where
>>> date is placed before time like '2008-02-14 20:30:45'.
>> Yes, it returns this -1 on other platforms, but...
>>> It must fail on
>>> other platforms as well.
>
> It also fails under Linux, but real problem is not here, it is just an
> unoptimal date parser.

The test does _not_ fail. That if condition does return -1 on Linux and BSD, and making tm_to_time_t() return a failure, but the caller goes on, ending up with the right values in year/month/date in the tm struct, which is the primary thing the function is used for.

As you said, what is_date() wants to see is if the caller guessed the order of three combinations of year/month/date correctly, it cannot necessarily assume the caller already has seen the hour/minutes/seconds yet, so _temporarily_ feeding a valud set of values to hour/minutes/seconds when calling tm_to_time_t() is a good workaround. So the change in your patch sounds correct and use of a temporary tm to avoid contaminating the hour/minutes/seconds passed to the is_date() function while doing so looks good.

Show 8 quoted lines

>> ... the input '2008-02-14 20:30:45' still parses fine for others
>> (including me).  That is what is puzzling.
>
>> A shot in the dark: perhaps your time_t is unsigned?
>
> Yeah, it was a headshot :) It really due to unsigned time_t. I will
> send two patches right now with fixes regarding unsigned time_t
> comparison.

If your time_t is unsigned, the error returned from tm_to_time_t() will appear to be a time in a distant future, which will prevent is_date() to return "Yeah, you guessed the order of year, month, and date correctly" to its caller. The code would need to pick a safer mechanism to signal a failure from tm_to_time_t() to its callers.