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, 18:26 UTC
- Message-ID
- <7vzjysxnb1.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <CAHXAxrOOqn6ZSVT1AFyO3a3paD1tokBtcnaX68a+ddhodOvZ6Q@mail.gmail.com>
Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:
> Fix is_date() function failings in detection of correct date in case > if time was not properly initialized.
Please explain why this patch is needed and what problem this patch is trying to fix (if any) a bit better in the proposed log message. For example, on what input do we call this function with partially filled *r, and is that an error in the code, or is it an indication that the input has only been consumed partially?
tm_to_time_t() is designed to reject underspecified date input, and its callers call is_date() starting with partially filled tm on purpose to reject such input. Doesn't "fixing" partially filled tm before calling tm_to_time_t() inside is_date() break that logic?
Show 38 quoted lines
> From: Mike Gorchak <mike.gorchak.qnx@gmail.com>
> Signed-off-by: Mike Gorchak <mike.gorchak.qnx@gmail.com>
> ---
> date.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/date.c b/date.c
> index 57331ed..ec758f4 100644
> --- a/date.c
> +++ b/date.c
> @@ -357,6 +357,7 @@ static int is_date(int year, int month, int day,
> struct tm *now_tm, time_t now,
> if (month > 0 && month < 13 && day > 0 && day < 32) {
> struct tm check = *tm;
> struct tm *r = (now_tm ? &check : tm);
> + struct tm fixed_r;
> time_t specified;
>
> r->tm_mon = month - 1;
> @@ -377,7 +378,16 @@ static int is_date(int year, int month, int day,
> struct tm *now_tm, time_t now,
> if (!now_tm)
> return 1;
>
> - specified = tm_to_time_t(r);
> + /* Fix tm structure in case if time was not initialized */
> + fixed_r = *r;
> + if (fixed_r.tm_hour==-1)
> + fixed_r.tm_hour=0;
> + if (fixed_r.tm_min==-1)
> + fixed_r.tm_min=0;
> + if (fixed_r.tm_sec==-1)
> + fixed_r.tm_sec=0;
> +
> + specified = tm_to_time_t(&fixed_r);
>
> /* Be it commit time or author time, it does not make
> * sense to specify timestamp way into the future. Make