Re: [PATCH 2/2] parse_date_basic(): let the system handle DST conversion

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Apr 15, 2015 at 12:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The function parses the input to compute the broken-down time in
> "struct tm", and the GMT timezone offset.  If the timezone offset
> does not exist in the input, the broken-down time is turned into the
> number of seconds since epoch both in the current timezone and in
> GMT and the offset is computed as their difference.
>
> However, we forgot to make sure tm.tm_isdst is set to -1 (i.e. let
> the system figure out if DST is in effect in the current timezone
> when turning the broken-down time to the number of seconds since
> epoch); it is done so at the beginning of the function, but a call
> to match_digit() in the function can lead to a call to gmtime_r() to
> clobber the field.

Thanks for composing the commit message and turning this into a proper patch.

> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Diagnosed-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

For what it's worth:

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

> ---
>  date.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/date.c b/date.c
> index 01fd73f..8ad6cef 100644
> --- a/date.c
> +++ b/date.c
> @@ -700,7 +700,11 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
>                 return -1;
>
>         if (*offset == -1) {
> -               time_t temp_time = mktime(&tm);
> +               time_t temp_time;
> +
> +               /* gmtime_r() in match_digit() may have clobbered it */
> +               tm.tm_isdst = -1;
> +               temp_time = mktime(&tm);
>                 if ((time_t)*timestamp > temp_time) {
>                         *offset = ((time_t)*timestamp - temp_time) / 60;
>                 } else {
> --
> 2.4.0-rc2-165-g862640d