git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Paul Eggert <eggert@cs.ucla.edu>, Eric Wong <e@80x24.org>,
	Taylor Blau <me@ttaylorr.com>
Subject: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)
Date: Mon, 20 Mar 2023 16:05:07 -0700	[thread overview]
Message-ID: <20230320230507.3932018-1-gitster@pobox.com> (raw)
In-Reply-To: <20230319064353.686226-1-eggert@cs.ucla.edu>

From: Paul Eggert <eggert@cs.ucla.edu>

Use gettimeofday instead of time(NULL) to get current time.
This avoids clock skew on glibc 2.31+ on Linux, where in the
first 1 to 2.5 ms of every second, time(NULL) returns a
value that is one less than the tv_sec part of
higher-resolution timestamps such as those returned by
gettimeofday or timespec_get, or those in the file system.
There are similar clock skew problems on AIX and MS-Windows,
which have problems in the first 5 ms of every second.

Without this patch, users can observe Git issuing a
timestamp T+1 before it issues timestamp T, because Git
sometimes uses time(NULL) or time(&t) and sometimes uses
higher-res methods like gettimeofday.  Although strictly
speaking users should tolerate this behavuior because a
superuser can always change the clock back, this is a
quality of implementation issue and users naturally expect
Git to issue timestamps in increasing order unless the
superuser has fiddled with the system clock.

This patch always uses gettimeofday(...) instead of time(...),
and I have verified that the resulting .o files never refer
to the name 'time'.  A trickier patch would change only
those calls for which timestamp monotonicity is user-visible.
Such a patch would require more expertise about Git internals,
though, and would be harder to maintain later.

Another possibility would be to change Git's documentation
to warn users that Git does not always issue timestamps in
increasing order.  However, Git users would likely be either
dismayed by this possibility, or confused by the level of
detail that any such documentation would require.

Yet another possibility would be to fix the Linux kernel so
that the time syscall is consistent with the other timestamp
syscalls.  I suppose this has not been done due to
performance implications.  (Git's use of timestamps is rare
enough that performance is not a significant consideration
for git.)  However, this wouldn't fix Git's problem on older
Linux kernels, or on AIX or MS-Windows.

Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This one is closer to v1 than my "how about" illustration, in
   that we use gettimeofday() everywhere, so I kept the authorship
   ident and timestamp from the original.

 git-compat-util.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 4f0028ce60..d6fbe9311d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -339,6 +339,25 @@ static inline const char *precompose_string_if_needed(const char *in)
 int compat_mkdir_wo_trailing_slash(const char*, mode_t);
 #endif
 
+#ifdef time
+#undef time
+#endif
+static inline time_t git_time(time_t *tloc)
+{
+	struct timeval tv;
+
+	/*
+	 * Avoid time(NULL), which can disagree with gettimeofday(2)
+	 * and filesystem timestamps.
+	 */
+	gettimeofday(&tv, NULL);
+
+	if (tloc)
+		*tloc = tv.tv_sec;
+	return tv.tv_sec;
+}
+#define time(x) git_time(x)
+
 #ifdef NO_STRUCT_ITIMERVAL
 struct itimerval {
 	struct timeval it_interval;
-- 
2.40.0-115-ge25cabbf6b


  parent reply	other threads:[~2023-03-20 23:05 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-19  6:43 [PATCH 0/2] use gettimeofday for current time Paul Eggert
2023-03-19  6:43 ` [PATCH 1/2] git-compat-util: time_now " Paul Eggert
2023-03-19  6:43 ` [PATCH 2/2] git-compat-util: use gettimeofday " Paul Eggert
2023-03-19 19:34   ` Eric Wong
2023-03-20 16:33     ` Junio C Hamano
2023-03-20 17:01       ` Junio C Hamano
2023-03-20 19:00         ` Paul Eggert
2023-03-20 19:40           ` Junio C Hamano
2023-03-20 20:36             ` Taylor Blau
2023-03-20 20:35       ` Taylor Blau
2023-03-20 23:05 ` Junio C Hamano [this message]
2023-03-20 23:21   ` [PATCH v2] git-compat-util: use gettimeofday(2) for time(2) Paul Eggert
2023-03-21 16:20     ` Junio C Hamano
2023-03-21 17:44   ` Konstantin Khomoutov
2023-03-21 18:22     ` Junio C Hamano
2023-03-21 18:22   ` Jeff King
2023-03-21 19:06     ` Taylor Blau
2023-03-21 20:04     ` Junio C Hamano
2023-03-22 17:11       ` Jeff King

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=20230320230507.3932018-1-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=e@80x24.org \
    --cc=eggert@cs.ucla.edu \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).