public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Eric Sunshine <sunshine@sunshineco•com>
Cc: Linus Torvalds <torvalds@linux-foundation•org>,
	Git Mailing List <git@vger•kernel.org>
Subject: Re: Odd broken "--date=now" behavior in current git
Date: Wed, 15 Apr 2015 07:42:41 -0700	[thread overview]
Message-ID: <xmqqk2xdmp0e.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150415072223.GA1389@flurp.local> (Eric Sunshine's message of "Wed, 15 Apr 2015 03:22:23 -0400")

Eric Sunshine <sunshine@sunshineco•com> writes:

> Later parse_date_basic() computes the offset from GMT by comparing
> the values returned by tm_to_time_t() and mktime(). The existing 'tm'
> is passed to mktime() with the tm_isdst field already set to 0 by
> gmtime_r(), and mktime() respects that as a statement that DST is not
> in effect, rather than determining it dynamically.
>
> The fix seems to be simply:
>
> ---- >8 ----
> diff --git a/date.c b/date.c
> index 3eba2df..99ad2a0 100644
> --- a/date.c
> +++ b/date.c
> @@ -707,6 +707,7 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
>  	/* mktime uses local timezone */
>  	*timestamp = tm_to_time_t(&tm);
>  	if (*offset == -1) {
> +		tm.tm_isdst = -1;
>  		time_t temp_time = mktime(&tm);
>  		if ((time_t)*timestamp > temp_time) {
>  			*offset = ((time_t)*timestamp - temp_time) / 60;
> ---- >8 ----

I briefly wondered if the caller of gmtime_r() in match_digit()
should be preserving the tm_isdst, though, as that codepath knows
that it is handling a bare number without GMT offset.

But resetting it to -1 here makes it even less error prone (we may
gain other code that stomp on tm.tm_isdst before we get here, and
having -1 in *offset is a sign that nobody saw GMT offset in the
input).

I think I see a decl-after-statment, but other than that, this looks
like a good fix.

Thanks.

next prev parent reply	other threads:[~2015-04-15 14:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-15  4:18 Odd broken "--date=now" behavior in current git Linus Torvalds
2015-04-15  4:47 ` Junio C Hamano
2015-04-15  7:22   ` Eric Sunshine
2015-04-15 14:42     ` Junio C Hamano [this message]
2015-04-15 16:21       ` [PATCH 1/2] parse_date_basic(): return early when given a bogus timestamp Junio C Hamano
2015-04-15 16:24       ` [PATCH 2/2] parse_date_basic(): let the system handle DST conversion Junio C Hamano
2015-04-15 17:23         ` Eric Sunshine
2015-04-15 16:20     ` Odd broken "--date=now" behavior in current git Linus Torvalds
2015-04-15 17:04       ` Junio C Hamano
2015-04-15  7:07 ` Peter Krefting

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk2xdmp0e.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=sunshine@sunshineco$(echo .)com \
    --cc=torvalds@linux-foundation$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox