From: Eric Sunshine Date: Wed, 15 Apr 2015 17:23:51 GMT Subject: Re: [PATCH 2/2] parse_date_basic(): let the system handle DST conversion Message-ID: In-Reply-To: On Wed, Apr 15, 2015 at 12:24 PM, Junio C Hamano 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 > Diagnosed-by: Eric Sunshine > Signed-off-by: Junio C Hamano For what it's worth: Reviewed-by: Eric Sunshine > --- > 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