Re: [PATCH 2/2] parse_date_basic(): let the system handle DST conversion
On Wed, Apr 15, 2015 at 12:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> 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>
Show 23 quoted lines
> ---
> 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-g862640dEric Sunshine <sunshine@sunshineco.com>