Re: [PATCH 2/3] Fix get_tz_offset to properly handle DST boundary cases

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2013, 22:58 UTC
Message-ID: <7vip61umbj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1360446418-12280-3-git-send-email-bdwalton@gmail.com>

Ben Walton <bdwalton@gmail.com> writes:

Show 10 quoted lines

> When passed a local time that was on the boundary of a DST change,
> get_tz_offset returned a GMT offset that was incorrect (off by one
> hour).  This is because the time was converted to GMT and then back to
> a time stamp via timelocal() which cannot disambiguate boundary cases
> as noted in its documentation.
>
> Modify this algorithm, using an approach suggested by Junio C Hamano
> that obtains the GMT time stamp by using timegm(localtime()) instead
> of timelocal(gmtime()).  This avoids the ambigious conversion and
> allows a correct time to be returned on every occassion.

I'll reword the log message a bit to explain why the updated logic is right and also refer to the message that has the suggestion. o

The implemmentation is a bit dense to my taste, but looks correct (I had to think about the comparison to come up with the value of the $sign, though).

Thanks.

Show 28 quoted lines

> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> ---
>  perl/Git.pm |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 5649bcc..a56d1e7 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -103,7 +103,7 @@ use Error qw(:try);
>  use Cwd qw(abs_path cwd);
>  use IPC::Open2 qw(open2);
>  use Fcntl qw(SEEK_SET SEEK_CUR);
> -use Time::Local qw(timelocal);
> +use Time::Local qw(timegm);
>  }
>  
>  
> @@ -528,8 +528,8 @@ If TIME is not supplied, the current local time is used.
>  sub get_tz_offset {
>  	# some systmes don't handle or mishandle %z, so be creative.
>  	my $t = shift || time;
> -	my $gm = timelocal(gmtime($t));
> -	my $sign = qw( + + - )[ $t <=> $gm ];
> +	my $gm = timegm(localtime($t));
> +	my $sign = qw( + + - )[ $gm <=> $t ];
>  	return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
>  }