{"thread":{"id":"59413","subject":"[PATCH 0/2] use gettimeofday for current time","startedAt":"2023-03-19T06:53:20Z","lastAt":"2023-03-22T17:12:04Z","messageCount":19,"participants":["Paul Eggert","Eric Wong","Junio C Hamano","Taylor Blau","Konstantin Khomoutov","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"473724","messageId":"20230319064353.686226-1-eggert@cs.ucla.edu","threadId":"59413","inReplyTo":null,"subject":"[PATCH 0/2] use gettimeofday for current time","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2023-03-19T06:43:51Z","receivedAt":"2023-03-19T06:53:20Z","isPatch":true,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"In recent GNU/Linux versions on rare occasions the 'time' function\nreturns a value that is one second less than the seconds component of\nthe timestamp that would be returned by gettimeofday, clock_gettime\nwith CLOCK_REALTIME, 'stat' on a recently modified file, etc.\n\nIf a program calls both 'time' and any of these other functions, the\nsystem clock will appear to jump backwards by a second temporarily.\nThis can lead to confusing or inconsistent output.\n\nA simple workaround for Git is to use 'gettimeofday' instead of\n'time'.  Although other options are possible (see the commit message\nof patch 0002), simplest is probably best.\n\nPaul Eggert (2):\n  git-compat-util: time_now for current time\n  git-compat-util: use gettimeofday for current time\n\n archive.c                          | 2 +-\n blame.c                            | 2 +-\n builtin/bugreport.c                | 2 +-\n builtin/credential-cache--daemon.c | 4 ++--\n builtin/diagnose.c                 | 2 +-\n builtin/fast-import.c              | 2 +-\n builtin/fsmonitor--daemon.c        | 4 ++--\n builtin/gc.c                       | 2 +-\n builtin/log.c                      | 2 +-\n builtin/receive-pack.c             | 2 +-\n builtin/reflog.c                   | 2 +-\n commit-graph.c                     | 4 ++--\n compat/nedmalloc/malloc.c.h        | 2 +-\n credential.c                       | 4 ++--\n date.c                             | 6 +++---\n git-compat-util.h                  | 8 ++++++++\n http-backend.c                     | 2 +-\n http-push.c                        | 6 +++---\n rerere.c                           | 2 +-\n run-command.c                      | 4 ++--\n t/helper/test-chmtime.c            | 4 ++--\n t/helper/test-simple-ipc.c         | 4 ++--\n 22 files changed, 40 insertions(+), 32 deletions(-)\n\n--\n2.39.2\n\n"},{"id":"473725","messageId":"20230319064353.686226-3-eggert@cs.ucla.edu","threadId":"59413","inReplyTo":"20230319064353.686226-1-eggert@cs.ucla.edu","subject":"[PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2023-03-19T06:43:53Z","receivedAt":"2023-03-19T06:53:22Z","isPatch":true,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"Use gettimeofday instead of time(NULL) to get current time.\nThis avoids clock skew on glibc 2.31+ on Linux, where in the\nfirst 1 to 2.5 ms of every second, time(NULL) returns a\nvalue that is one less than the tv_sec part of\nhigher-resolution timestamps such as those returned by\ngettimeofday or timespec_get, or those in the file system.\nThere are similar clock skew problems on AIX and MS-Windows,\nwhich have problems in the first 5 ms of every second.\n\nWithout this patch, users can observe Git issuing a\ntimestamp T+1 before it issues timestamp T, because Git\nsometimes uses time(NULL) or time(&t) and sometimes uses\nhigher-res methods like gettimeofday.  Although strictly\nspeaking users should tolerate this behavuior because a\nsuperuser can always change the clock back, this is a\nquality of implementation issue and users naturally expect\nGit to issue timestamps in increasing order unless the\nsuperuser has fiddled with the system clock.\n\nThis patch always uses gettimeofday(...) instead of time(...),\nand I have verified that the resulting .o files never refer\nto the name 'time'.  A trickier patch would change only\nthose calls for which timestamp monotonicity is user-visible.\nSuch a patch would require more expertise about Git internals,\nthough, and would be harder to maintain later.\n\nAnother possibility would be to change Git's documentation\nto warn users that Git does not always issue timestamps in\nincreasing order.  However, Git users would likely be either\ndismayed by this possibility, or confused by the level of\ndetail that any such documentation would require.\n\nYet another possibility would be to fix the Linux kernel so\nthat the time syscall is consistent with the other timestamp\nsyscalls.  I suppose this has not been done due to\nperformance implications.  (Git's use of timestamps is rare\nenough that performance is not a significant consideration\nfor git.)  However, this wouldn't fix Git's problem on older\nLinux kernels, or on AIX or MS-Windows.\n\nSigned-off-by: Paul Eggert <eggert@cs.ucla.edu>\n---\n git-compat-util.h | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex d05f4bac22..dcd9e5ca65 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -341,7 +341,10 @@ int compat_mkdir_wo_trailing_slash(const char*, mode_t);\n \n static inline time_t time_now(void)\n {\n-\treturn time(NULL);\n+\t/* Avoid time(NULL), which can disagree with gettimeofday etc.  */\n+\tstruct timeval tv;\n+\tgettimeofday(&tv, NULL);\n+\treturn tv.tv_sec;\n }\n \n #ifdef NO_STRUCT_ITIMERVAL\n-- \n2.39.2\n\n"},{"id":"473726","messageId":"20230319064353.686226-2-eggert@cs.ucla.edu","threadId":"59413","inReplyTo":"20230319064353.686226-1-eggert@cs.ucla.edu","subject":"[PATCH 1/2] git-compat-util: time_now for current time","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2023-03-19T06:43:52Z","receivedAt":"2023-03-19T06:53:26Z","isPatch":true,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"Add a function time_now to get the current time as a time_t value.\nAll uses of time(NULL) or time(&xxx) changed to use this new function.\nThis refactoring does not change behavior.\n\nSigned-off-by: Paul Eggert <eggert@cs.ucla.edu>\n---\n archive.c                          | 2 +-\n blame.c                            | 2 +-\n builtin/bugreport.c                | 2 +-\n builtin/credential-cache--daemon.c | 4 ++--\n builtin/diagnose.c                 | 2 +-\n builtin/fast-import.c              | 2 +-\n builtin/fsmonitor--daemon.c        | 4 ++--\n builtin/gc.c                       | 2 +-\n builtin/log.c                      | 2 +-\n builtin/receive-pack.c             | 2 +-\n builtin/reflog.c                   | 2 +-\n commit-graph.c                     | 4 ++--\n compat/nedmalloc/malloc.c.h        | 2 +-\n credential.c                       | 4 ++--\n date.c                             | 6 +++---\n git-compat-util.h                  | 5 +++++\n http-backend.c                     | 2 +-\n http-push.c                        | 6 +++---\n rerere.c                           | 2 +-\n run-command.c                      | 4 ++--\n t/helper/test-chmtime.c            | 4 ++--\n t/helper/test-simple-ipc.c         | 4 ++--\n 22 files changed, 37 insertions(+), 32 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 1c2ca78e52..5cdafc9b56 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -472,7 +472,7 @@ static void parse_treeish_arg(const char **argv,\n \t\tarchive_time = commit->date;\n \t} else {\n \t\tcommit_oid = NULL;\n-\t\tarchive_time = time(NULL);\n+\t\tarchive_time = time_now();\n \t}\n \tif (ar_args->mtime_option)\n \t\tarchive_time = approxidate(ar_args->mtime_option);\ndiff --git a/blame.c b/blame.c\nindex e45d8a3bf9..df14c1dfe4 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -192,7 +192,7 @@ static struct commit *fake_working_tree_commit(struct repository *r,\n \tstruct strbuf msg = STRBUF_INIT;\n \n \trepo_read_index(r);\n-\ttime(&now);\n+\tnow = time_now();\n \tcommit = alloc_commit_node(r);\n \tcommit->object.parsed = 1;\n \tcommit->date = now;\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 5bc254be80..bba5820787 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -98,7 +98,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buffer = STRBUF_INIT;\n \tstruct strbuf report_path = STRBUF_INIT;\n \tint report = -1;\n-\ttime_t now = time(NULL);\n+\ttime_t now = time_now();\n \tstruct tm tm;\n \tenum diagnose_mode diagnose = DIAGNOSE_NONE;\n \tchar *option_output = NULL;\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex 6e509d02c3..7a87df16d2 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -27,7 +27,7 @@ static void cache_credential(struct credential *c, int timeout)\n \t/* take ownership of pointers */\n \tmemcpy(&e->item, c, sizeof(*c));\n \tmemset(c, 0, sizeof(*c));\n-\te->expiration = time(NULL) + timeout;\n+\te->expiration = time_now() + timeout;\n }\n \n static struct credential_cache_entry *lookup_credential(const struct credential *c)\n@@ -54,7 +54,7 @@ static timestamp_t check_expirations(void)\n {\n \tstatic timestamp_t wait_for_entry_until;\n \tint i = 0;\n-\ttimestamp_t now = time(NULL);\n+\ttimestamp_t now = time_now();\n \ttimestamp_t next = TIME_MAX;\n \n \t/*\ndiff --git a/builtin/diagnose.c b/builtin/diagnose.c\nindex d52015c67a..c502096e6c 100644\n--- a/builtin/diagnose.c\n+++ b/builtin/diagnose.c\n@@ -11,7 +11,7 @@ static const char * const diagnose_usage[] = {\n int cmd_diagnose(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf zip_path = STRBUF_INIT;\n-\ttime_t now = time(NULL);\n+\ttime_t now = time_now();\n \tstruct tm tm;\n \tenum diagnose_mode mode = DIAGNOSE_STATS;\n \tchar *option_output = NULL;\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex f7548ff4a3..9096f52075 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -335,7 +335,7 @@ static void write_crash_report(const char *err)\n \tfprintf(rpt, \"fast-import crash report:\\n\");\n \tfprintf(rpt, \"    fast-import process: %\"PRIuMAX\"\\n\", (uintmax_t) getpid());\n \tfprintf(rpt, \"    parent process     : %\"PRIuMAX\"\\n\", (uintmax_t) getppid());\n-\tfprintf(rpt, \"    at %s\\n\", show_date(time(NULL), 0, DATE_MODE(ISO8601)));\n+\tfprintf(rpt, \"    at %s\\n\", show_date(time_now(), 0, DATE_MODE(ISO8601)));\n \tfputc('\\n', rpt);\n \n \tfputs(\"fatal: \", rpt);\ndiff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c\nindex cae804a190..45cc8134cc 100644\n--- a/builtin/fsmonitor--daemon.c\n+++ b/builtin/fsmonitor--daemon.c\n@@ -415,7 +415,7 @@ static struct fsmonitor_token_data *fsmonitor_new_token_data(void)\n \t * events to accumulate.\n \t */\n \tif (test_env_value)\n-\t\tbatch->pinned_time = time(NULL);\n+\t\tbatch->pinned_time = time_now();\n \n \treturn token;\n }\n@@ -772,7 +772,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,\n \t */\n \ttoken_data = state->current_token_data;\n \tbatch_head = token_data->batch_head;\n-\t((struct fsmonitor_batch *)batch_head)->pinned_time = time(NULL);\n+\t((struct fsmonitor_batch *)batch_head)->pinned_time = time_now();\n \n \t/*\n \t * FSMonitor Protocol V2 requires that we send a response header\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c58fe8c936..029bd2ba5c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -450,7 +450,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \t\t\t * after the user verifies that no gc is\n \t\t\t * running.\n \t\t\t */\n-\t\t\ttime(NULL) - st.st_mtime <= 12 * 3600 &&\n+\t\t\ttime_now() - st.st_mtime <= 12 * 3600 &&\n \t\t\tfscanf(fp, scan_fmt, &pid, locking_host) == 2 &&\n \t\t\t/* be gentle to concurrent \"gc\" on remote hosts */\n \t\t\t(strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM);\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b62e629ba8..bdc62d9b9e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1168,7 +1168,7 @@ static void gen_message_id(struct rev_info *info, char *base)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstrbuf_addf(&buf, \"%s.%\"PRItime\".git.%s\", base,\n-\t\t    (timestamp_t) time(NULL),\n+\t\t    (timestamp_t) time_now(),\n \t\t    git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE|IDENT_STRICT));\n \tinfo->message_id = strbuf_detach(&buf, NULL);\n }\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex fe68c79ee3..3fedeea65c 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2504,7 +2504,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \n \tgit_config(receive_pack_config, NULL);\n \tif (cert_nonce_seed)\n-\t\tpush_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL));\n+\t\tpush_cert_nonce = prepare_push_cert_nonce(service_dir, time_now());\n \n \tif (0 <= transfer_unpack_limit)\n \t\tunpack_limit = transfer_unpack_limit;\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 270681dcdf..013673e768 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -237,7 +237,7 @@ static int cmd_reflog_show(int argc, const char **argv, const char *prefix)\n static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n {\n \tstruct cmd_reflog_expire_cb cmd = { 0 };\n-\ttimestamp_t now = time(NULL);\n+\ttimestamp_t now = time_now();\n \tint i, status, do_all, all_worktrees = 1;\n \tunsigned int flags = 0;\n \tint verbose = 0;\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 5e6098ff35..4c75d96fc4 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -2196,7 +2196,7 @@ static void merge_commit_graphs(struct write_commit_graph_context *ctx)\n static void mark_commit_graphs(struct write_commit_graph_context *ctx)\n {\n \tuint32_t i;\n-\ttime_t now = time(NULL);\n+\ttime_t now = time_now();\n \n \tfor (i = ctx->num_commit_graphs_after - 1; i < ctx->num_commit_graphs_before; i++) {\n \t\tstruct stat st;\n@@ -2217,7 +2217,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)\n \tDIR *dir;\n \tstruct dirent *de;\n \tsize_t dirnamelen;\n-\ttimestamp_t expire_time = time(NULL);\n+\ttimestamp_t expire_time = time_now();\n \n \tif (ctx->opts && ctx->opts->expire_time)\n \t\texpire_time = ctx->opts->expire_time;\ndiff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h\nindex 814845d4b3..4c30a49e60 100644\n--- a/compat/nedmalloc/malloc.c.h\n+++ b/compat/nedmalloc/malloc.c.h\n@@ -3085,7 +3085,7 @@ static int init_mparams(void) {\n #ifdef WIN32\n \tmagic = (size_t)(GetTickCount() ^ (size_t)0x55555555U);\n #else\n-      magic = (size_t)(time(0) ^ (size_t)0x55555555U);\n+      magic = (size_t)(time_now() ^ (size_t)0x55555555U);\n #endif\n       magic |= (size_t)8U;    /* ensure nonzero */\n       magic &= ~(size_t)7U;   /* improve chances of fault for bad values */\ndiff --git a/credential.c b/credential.c\nindex ea40a2a410..dc750fa502 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -356,7 +356,7 @@ void credential_fill(struct credential *c)\n \n \tfor (i = 0; i < c->helpers.nr; i++) {\n \t\tcredential_do(c, c->helpers.items[i].string, \"get\");\n-\t\tif (c->password_expiry_utc < time(NULL)) {\n+\t\tif (c->password_expiry_utc < time_now()) {\n \t\t\t/* Discard expired password */\n \t\t\tFREE_AND_NULL(c->password);\n \t\t\t/* Reset expiry to maintain consistency */\n@@ -380,7 +380,7 @@ void credential_approve(struct credential *c)\n \n \tif (c->approved)\n \t\treturn;\n-\tif (!c->username || !c->password || c->password_expiry_utc < time(NULL))\n+\tif (!c->username || !c->password || c->password_expiry_utc < time_now())\n \t\treturn;\n \n \tcredential_apply_config(c);\ndiff --git a/date.c b/date.c\nindex 6f45eeb356..16717be397 100644\n--- a/date.c\n+++ b/date.c\n@@ -597,7 +597,7 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase '/':\n \tcase '.':\n \t\tif (!now)\n-\t\t\tnow = time(NULL);\n+\t\t\tnow = time_now();\n \t\trefuse_future = NULL;\n \t\tif (gmtime_r(&now, &now_tm))\n \t\t\trefuse_future = &now_tm;\n@@ -712,7 +712,7 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tunsigned int num2 = (num % 10000) / 100;\n \t\tunsigned int num3 = num % 100;\n \t\tif (n == 8)\n-\t\t\tset_date(num1, num2, num3, NULL, time(NULL), tm);\n+\t\t\tset_date(num1, num2, num3, NULL, time_now(), tm);\n \t\telse if (n == 6 && set_time(num1, num2, num3, tm) == 0 &&\n \t\t\t *end == '.' && isdigit(end[1]))\n \t\t\tstrtoul(end + 1, &end, 10);\n@@ -1041,7 +1041,7 @@ void datestamp(struct strbuf *out)\n \tint offset;\n \tstruct tm tm = { 0 };\n \n-\ttime(&now);\n+\tnow = time_now();\n \n \toffset = tm_to_time_t(localtime_r(&now, &tm)) - now;\n \toffset /= 60;\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 1e6592624d..d05f4bac22 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -339,6 +339,11 @@ static inline const char *precompose_string_if_needed(const char *in)\n int compat_mkdir_wo_trailing_slash(const char*, mode_t);\n #endif\n \n+static inline time_t time_now(void)\n+{\n+\treturn time(NULL);\n+}\n+\n #ifdef NO_STRUCT_ITIMERVAL\n struct itimerval {\n \tstruct timeval it_interval;\ndiff --git a/http-backend.c b/http-backend.c\nindex 9cfc6f2541..d533989e32 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -113,7 +113,7 @@ static void hdr_nocache(struct strbuf *hdr)\n \n static void hdr_cache_forever(struct strbuf *hdr)\n {\n-\ttimestamp_t now = time(NULL);\n+\ttimestamp_t now = time_now();\n \thdr_date(hdr, \"Date\", now);\n \thdr_date(hdr, \"Expires\", now + 31536000);\n \thdr_str(hdr, \"Cache-Control\", \"public, max-age=31536000\");\ndiff --git a/http-push.c b/http-push.c\nindex 88aa045ecb..d8158667a0 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -459,7 +459,7 @@ static int refresh_lock(struct remote_lock *lock)\n \t\t\tfprintf(stderr, \"LOCK HTTP error %ld\\n\",\n \t\t\t\tresults.http_code);\n \t\t} else {\n-\t\t\tlock->start_time = time(NULL);\n+\t\t\tlock->start_time = time_now();\n \t\t\trc = 1;\n \t\t}\n \t}\n@@ -473,7 +473,7 @@ static int refresh_lock(struct remote_lock *lock)\n static void check_locks(void)\n {\n \tstruct remote_lock *lock = repo->locks;\n-\ttime_t current_time = time(NULL);\n+\ttime_t current_time = time_now();\n \tint time_remaining;\n \n \twhile (lock) {\n@@ -933,7 +933,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \t\tFREE_AND_NULL(lock);\n \t} else {\n \t\tlock->url = url;\n-\t\tlock->start_time = time(NULL);\n+\t\tlock->start_time = time_now();\n \t\tlock->next = repo->locks;\n \t\trepo->locks = lock;\n \t}\ndiff --git a/rerere.c b/rerere.c\nindex a67abaab07..ac36a9c234 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1179,7 +1179,7 @@ void rerere_gc(struct repository *r, struct string_list *rr)\n \tDIR *dir;\n \tstruct dirent *e;\n \tint i;\n-\ttimestamp_t now = time(NULL);\n+\ttimestamp_t now = time_now();\n \ttimestamp_t cutoff_noresolve = now - 15 * 86400;\n \ttimestamp_t cutoff_resolve = now - 60 * 86400;\n \ndiff --git a/run-command.c b/run-command.c\nindex ba617655b2..fbf9afdb27 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1864,7 +1864,7 @@ enum start_bg_result start_bg_command(struct child_process *cmd,\n \t\tgoto done;\n \t}\n \n-\ttime(&time_limit);\n+\ttime_limit = time_now();\n \ttime_limit += timeout_sec;\n \n wait:\n@@ -1891,7 +1891,7 @@ enum start_bg_result start_bg_command(struct child_process *cmd,\n \t\t\t */\n \t\t\ttime_t now;\n \n-\t\t\ttime(&now);\n+\t\t\tnow = time_now();\n \t\t\tif (now < time_limit)\n \t\t\t\tgoto wait;\n \ndiff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c\nindex dc28890a18..f48a07f6f7 100644\n--- a/t/helper/test-chmtime.c\n+++ b/t/helper/test-chmtime.c\n@@ -7,7 +7,7 @@\n  *\n  *\ttest-tool chmtime =<seconds> file...\n  *\n- * Relative to the current time as returned by time(3):\n+ * Relative to the current time as returned by time_now():\n  *\n  *\ttest-tool chmtime =+<seconds> (or =-<seconds>) file...\n  *\n@@ -60,7 +60,7 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)\n \t\treturn 0;\n \t}\n \tif ((*set_eq && *set_time < 0) || *set_eq == 2) {\n-\t\ttime_t now = time(NULL);\n+\t\ttime_t now = time_now();\n \t\t*set_time += now;\n \t}\n \treturn 1;\ndiff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c\nindex 3d1436da59..2267cd9fac 100644\n--- a/t/helper/test-simple-ipc.c\n+++ b/t/helper/test-simple-ipc.c\n@@ -411,7 +411,7 @@ static int client__stop_server(void)\n \ttime_t time_limit, now;\n \tenum ipc_active_state s;\n \n-\ttime(&time_limit);\n+\ttime_limit = time_now();\n \ttime_limit += cl_args.max_wait_sec;\n \n \tcl_args.token = \"quit\";\n@@ -434,7 +434,7 @@ static int client__stop_server(void)\n \t\t\treturn 0;\n \t\t}\n \n-\t\ttime(&now);\n+\t\tnow = time_now();\n \t\tif (now > time_limit)\n \t\t\treturn error(\"daemon has not shutdown yet\");\n \t}\n-- \n2.39.2\n\n"},{"id":"473741","messageId":"20230319193449.M629601@dcvr","threadId":"59413","inReplyTo":"20230319064353.686226-3-eggert@cs.ucla.edu","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2023-03-19T19:34:49Z","receivedAt":"2023-03-19T19:34:57Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Paul Eggert <eggert@cs.ucla.edu> wrote:\n> Use gettimeofday instead of time(NULL) to get current time.\n> This avoids clock skew on glibc 2.31+ on Linux, where in the\n> first 1 to 2.5 ms of every second, time(NULL) returns a\n> value that is one less than the tv_sec part of\n> higher-resolution timestamps such as those returned by\n> gettimeofday or timespec_get, or those in the file system.\n> There are similar clock skew problems on AIX and MS-Windows,\n> which have problems in the first 5 ms of every second.\n\nWow, this is enlightening... and unfortunate :<\n\nI decided to check glibc archives to find more discussion on it.\nSo far, I've found:\n\n  https://inbox.sourceware.org/libc-alpha/20230306160321.2942372-1-adhemerval.zanella@linaro.org/T/\n\nand the original bug:\n  https://sourceware.org/bugzilla/show_bug.cgi?id=30200\n\nAnd this is due to the time64 changes in glibc 2.31+?\n(<= 2.30 isn't affected?)\n\n<snip>\n\n> Yet another possibility would be to fix the Linux kernel so\n> that the time syscall is consistent with the other timestamp\n> syscalls.  I suppose this has not been done due to\n> performance implications.  (Git's use of timestamps is rare\n> enough that performance is not a significant consideration\n> for git.)  However, this wouldn't fix Git's problem on older\n> Linux kernels, or on AIX or MS-Windows.\n\nAgreed on the older kernels and other OSes part.\n"},{"id":"473782","messageId":"xmqqh6ufo01u.fsf@gitster.g","threadId":"59413","inReplyTo":"20230319193449.M629601@dcvr","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-20T16:33:01Z","receivedAt":"2023-03-20T16:39:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n> Paul Eggert <eggert@cs.ucla.edu> wrote:\n>> Use gettimeofday instead of time(NULL) to get current time.\n>> This avoids clock skew on glibc 2.31+ on Linux, where in the\n>> first 1 to 2.5 ms of every second, time(NULL) returns a\n>> value that is one less than the tv_sec part of\n>> higher-resolution timestamps such as those returned by\n>> gettimeofday or timespec_get, or those in the file system.\n>> There are similar clock skew problems on AIX and MS-Windows,\n>> which have problems in the first 5 ms of every second.\n>\n> Wow, this is enlightening... and unfortunate :<\n>\n> I decided to check glibc archives to find more discussion on it.\n> So far, I've found:\n>\n>   https://inbox.sourceware.org/libc-alpha/20230306160321.2942372-1-adhemerval.zanella@linaro.org/T/\n>\n> and the original bug:\n>   https://sourceware.org/bugzilla/show_bug.cgi?id=30200\n>\n> And this is due to the time64 changes in glibc 2.31+?\n> (<= 2.30 isn't affected?)\n>\n> <snip>\n>\n>> Yet another possibility would be to fix the Linux kernel so\n>> that the time syscall is consistent with the other timestamp\n>> syscalls.  I suppose this has not been done due to\n>> performance implications.  (Git's use of timestamps is rare\n>> enough that performance is not a significant consideration\n>> for git.)  However, this wouldn't fix Git's problem on older\n>> Linux kernels, or on AIX or MS-Windows.\n>\n> Agreed on the older kernels and other OSes part.\n\nYeah, this is interesting.  I however wonder if we should follow our\nusual pattern of implementing git_time() with the identical function\nsignature as what we replace (i.e. system's time()), and #undef/#define\nthe symbol we replace with git_time, though.  Wouldn't it make [1/2]\na lot smaller and future-proof?\n\n"},{"id":"473784","messageId":"xmqqr0tjmk6d.fsf@gitster.g","threadId":"59413","inReplyTo":"xmqqh6ufo01u.fsf@gitster.g","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-20T17:01:14Z","receivedAt":"2023-03-20T17:08:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Eric Wong <e@80x24.org> writes:\n> ...\n>> I decided to check glibc archives to find more discussion on it.\n>> So far, I've found:\n>>\n>>   https://inbox.sourceware.org/libc-alpha/20230306160321.2942372-1-adhemerval.zanella@linaro.org/T/\n>>\n>> and the original bug:\n>>   https://sourceware.org/bugzilla/show_bug.cgi?id=30200\n>>\n>> And this is due to the time64 changes in glibc 2.31+?\n>> (<= 2.30 isn't affected?)\n>>\n> ...\n>\n> Yeah, this is interesting.  I however wonder if we should follow our\n> usual pattern of implementing git_time() with the identical function\n> signature as what we replace (i.e. system's time()), and #undef/#define\n> the symbol we replace with git_time, though.  Wouldn't it make [1/2]\n> a lot smaller and future-proof?\n\nAnd here is how such an approach may look like.  The second part is\nmore or less the same as Paul's version so I kept his authorship\nthere (these are mostly for discussion and not for application).\n\n----- >8 ----- [PATCH 1/2] ----- >8 -----\nSubject: [PATCH] time(): allow OVERRIDE_TIME to redefine time(2)\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile          | 11 +++++++++++\n compat/time.c     | 11 +++++++++++\n config.mak.uname  |  1 +\n git-compat-util.h |  8 ++++++++\n 4 files changed, 31 insertions(+)\n create mode 100644 compat/time.c\n\ndiff --git a/Makefile b/Makefile\nindex 50ee51fde3..aa6fcd6e04 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -286,6 +286,12 @@ include shared.mak\n # crashes due to allocation and free working on different 'heaps'.\n # It's defined automatically if USE_NED_ALLOCATOR is set.\n #\n+# Define OVERRIDE_TIME to override time(2) and replace it with an\n+# implementation based on gettimeofday(2).  THis is necessary when\n+# glibc 2.31+ on Linux is used, where in the first 1 to 2.5 ms of\n+# every second, time(NULL) returns a value that is one less than the\n+# tv_sec part of higher-resolution timestamps used in the file system.\n+#\n # Define NO_REGEX if your C library lacks regex support with REG_STARTEND\n # feature.\n #\n@@ -2061,6 +2067,11 @@ ifdef OVERRIDE_STRDUP\n \tCOMPAT_OBJS += compat/strdup.o\n endif\n \n+ifdef OVERRIDE_TIME\n+\tCOMPAT_CFLAGS += -DOVERRIDE_TIME\n+\tCOMPAT_OBJS += compat/time.o\n+endif\n+\n ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT\n \texport GIT_TEST_CMP_USE_COPIED_CONTEXT\n endif\ndiff --git a/compat/time.c b/compat/time.c\nnew file mode 100644\nindex 0000000000..a3ef5c0e98\n--- /dev/null\n+++ b/compat/time.c\n@@ -0,0 +1,11 @@\n+#include \"../git-compat-util.h\"\n+\n+#undef time\n+time_t git_time(time_t *tloc)\n+{\n+\ttime_t t = time(NULL);\n+\n+\tif (tloc)\n+\t\t*tloc = t;\n+\treturn t;\n+}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 64c44db805..29398919e8 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -64,6 +64,7 @@ ifeq ($(uname_S),Linux)\n \tPROCFS_EXECUTABLE_PATH = /proc/self/exe\n \tHAVE_PLATFORM_PROCINFO = YesPlease\n \tCOMPAT_OBJS += compat/linux/procinfo.o\n+\tOVERRIDE_TIME = YesPlease\n \t# centos7/rhel7 provides gcc 4.8.5 and zlib 1.2.7.\n \tifneq ($(findstring .el7.,$(uname_R)),)\n \t\tBASIC_CFLAGS += -std=c99\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 1e6592624d..2279f3c90c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -917,6 +917,14 @@ void *gitmemmem(const void *haystack, size_t haystacklen,\n char *gitstrdup(const char *s);\n #endif\n \n+#ifdef OVERRIDE_TIME\n+#ifdef time\n+#undef time\n+#endif\n+#define time git_time\n+extern time_t git_time(time_t *);\n+#endif\n+\n #ifdef NO_GETPAGESIZE\n #define getpagesize() sysconf(_SC_PAGESIZE)\n #endif\n-- \n2.40.0-115-ge25cabbf6b\n\n\n----- >8 ----- [PATCH 2/2] ----- >8 -----\nFrom: Paul Eggert <eggert@cs.ucla.edu>\nFrom: Paul Eggert <eggert@cs.ucla.edu>\nSubject: [PATCH] time(): use gettimeofday()\n\nUse gettimeofday instead of time(NULL) to get current time.\nThis avoids clock skew on glibc 2.31+ on Linux, where in the\nfirst 1 to 2.5 ms of every second, time(NULL) returns a\nvalue that is one less than the tv_sec part of\nhigher-resolution timestamps such as those returned by\ngettimeofday or timespec_get, or those in the file system.\nThere are similar clock skew problems on AIX and MS-Windows,\nwhich have problems in the first 5 ms of every second.\n\nWithout this patch, users can observe Git issuing a\ntimestamp T+1 before it issues timestamp T, because Git\nsometimes uses time(NULL) or time(&t) and sometimes uses\nhigher-res methods like gettimeofday.  Although strictly\nspeaking users should tolerate this behavuior because a\nsuperuser can always change the clock back, this is a\nquality of implementation issue and users naturally expect\nGit to issue timestamps in increasing order unless the\nsuperuser has fiddled with the system clock.\n\nThis patch always uses gettimeofday(...) instead of time(...),\nand I have verified that the resulting .o files never refer\nto the name 'time'.  A trickier patch would change only\nthose calls for which timestamp monotonicity is user-visible.\nSuch a patch would require more expertise about Git internals,\nthough, and would be harder to maintain later.\n\nAnother possibility would be to change Git's documentation\nto warn users that Git does not always issue timestamps in\nincreasing order.  However, Git users would likely be either\ndismayed by this possibility, or confused by the level of\ndetail that any such documentation would require.\n\nYet another possibility would be to fix the Linux kernel so\nthat the time syscall is consistent with the other timestamp\nsyscalls.  I suppose this has not been done due to\nperformance implications.  (Git's use of timestamps is rare\nenough that performance is not a significant consideration\nfor git.)  However, this wouldn't fix Git's problem on older\nLinux kernels, or on AIX or MS-Windows.\n\nSigned-off-by: Paul Eggert <eggert@cs.ucla.edu>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n compat/time.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/time.c b/compat/time.c\nindex a3ef5c0e98..758a160ee7 100644\n--- a/compat/time.c\n+++ b/compat/time.c\n@@ -3,9 +3,15 @@\n #undef time\n time_t git_time(time_t *tloc)\n {\n-\ttime_t t = time(NULL);\n+\t/*\n+\t * Avoid time(NULL), which can disagree with gettimeofday()\n+\t * and filesystem timestamps.\n+\t */\n+\tstruct timeval tv;\n+\n+\tgettimeofday(&tv, NULL);\n \n \tif (tloc)\n-\t\t*tloc = t;\n-\treturn t;\n+\t\t*tloc = tv.tv_sec;\n+\treturn tv.tv_sec;\n }\n-- \n2.40.0-115-ge25cabbf6b\n\n"},{"id":"473799","messageId":"047178a8-ccf5-1cab-e670-8f1c64f9ca3c@cs.ucla.edu","threadId":"59413","inReplyTo":"xmqqr0tjmk6d.fsf@gitster.g","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2023-03-20T19:00:22Z","receivedAt":"2023-03-20T19:17:01Z","isPatch":true,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"On 3/20/23 10:01, Junio C Hamano wrote:\n\n>> I however wonder if we should follow our\n>> usual pattern of implementing git_time() with the identical function\n>> signature as what we replace (i.e. system's time()), and #undef/#define\n>> the symbol we replace with git_time, though.  Wouldn't it make [1/2]\n>> a lot smaller and future-proof?\n\nYes, something like that would work too. (Sorry, I didn't know about the \nusual pattern.)\n\n\n> +# Define OVERRIDE_TIME to override time(2) and replace it with an\n> +# implementation based on gettimeofday(2).  THis is necessary when\n> +# glibc 2.31+ on Linux is used, where in the first 1 to 2.5 ms of\n> +# every second, time(NULL) returns a value that is one less than the\n> +# tv_sec part of higher-resolution timestamps used in the file system.\n\nTHis -> This\n\nIt might be simpler to use the gettimeofday workaround on all platforms, \nrather than having an OVERRIDE_TIME flag and complicating \nconfig.mak.uname. gettimeofday should be portable, as it's already used \nelsewhere in Git without configury.\n\nIf we're going with the more-complicated solution, config.mak.uname will \nneed changes in its AIX and MS-Windows sections because the problem is \nknown to occur there too. Although AIX configuration is simple, I'm not \nsure how to go about the MS-Windows part as there seem to be a lot of \nifdefs there. Also, because the time problem does not occur with musl \nlibc (used in Alpine Linux 3.17), on the Linux side the workaround could \nbe employed only if glibc 2.31+ is in use.\n\n"},{"id":"473801","messageId":"xmqqfs9zky94.fsf@gitster.g","threadId":"59413","inReplyTo":"047178a8-ccf5-1cab-e670-8f1c64f9ca3c@cs.ucla.edu","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-20T19:40:07Z","receivedAt":"2023-03-20T19:44:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Eggert <eggert@cs.ucla.edu> writes:\n\n> It might be simpler to use the gettimeofday workaround on all\n> platforms, rather than having an OVERRIDE_TIME flag and complicating\n> config.mak.uname. gettimeofday should be portable, as it's already\n> used elsewhere in Git without configury.\n\nThat is an excellent point.\n"},{"id":"473810","messageId":"ZBjDfT15489G1o3Q@nand.local","threadId":"59413","inReplyTo":"xmqqh6ufo01u.fsf@gitster.g","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-03-20T20:35:09Z","receivedAt":"2023-03-20T20:35:17Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Mar 20, 2023 at 09:33:01AM -0700, Junio C Hamano wrote:\n> Yeah, this is interesting.\n\nDefinitely so. You learn something new every day ;-).\n\n> I however wonder if we should follow our usual pattern of implementing\n> git_time() with the identical function signature as what we replace\n> (i.e. system's time()), and #undef/#define the symbol we replace with\n> git_time, though.  Wouldn't it make [1/2] a lot smaller and\n> future-proof?\n\nYeah, I agree here, too. It was my first thought when I started reading\nPaul's patches here. I think that your approach is sound and I would be\nhappy to see you queue it.\n\nThanks,\nTaylor\n"},{"id":"473811","messageId":"ZBjD2l5VhW/QhZXH@nand.local","threadId":"59413","inReplyTo":"xmqqfs9zky94.fsf@gitster.g","subject":"Re: [PATCH 2/2] git-compat-util: use gettimeofday for current time","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-03-20T20:36:42Z","receivedAt":"2023-03-20T20:37:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Mar 20, 2023 at 12:40:07PM -0700, Junio C Hamano wrote:\n> Paul Eggert <eggert@cs.ucla.edu> writes:\n>\n> > It might be simpler to use the gettimeofday workaround on all\n> > platforms, rather than having an OVERRIDE_TIME flag and complicating\n> > config.mak.uname. gettimeofday should be portable, as it's already\n> > used elsewhere in Git without configury.\n>\n> That is an excellent point.\n\nI'd be happy to assume OVERRIDE_TIME is set everywhere and just drop it\nentirely (using gettimeofday() unconditionally within git_time()).\n\nAn alternative approach might be to leave OVERRIDE_TIME time in place,\nbut treat it as opt-out instead of opt-in. I can imagine that some\nexotic platform might want to use time() instead of gettimeofday() for\none reason or another.\n\nThanks,\nTaylor\n"},{"id":"473818","messageId":"20230320230507.3932018-1-gitster@pobox.com","threadId":"59413","inReplyTo":"20230319064353.686226-1-eggert@cs.ucla.edu","subject":"[PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-20T23:05:07Z","receivedAt":"2023-03-20T23:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Paul Eggert <eggert@cs.ucla.edu>\n\nUse gettimeofday instead of time(NULL) to get current time.\nThis avoids clock skew on glibc 2.31+ on Linux, where in the\nfirst 1 to 2.5 ms of every second, time(NULL) returns a\nvalue that is one less than the tv_sec part of\nhigher-resolution timestamps such as those returned by\ngettimeofday or timespec_get, or those in the file system.\nThere are similar clock skew problems on AIX and MS-Windows,\nwhich have problems in the first 5 ms of every second.\n\nWithout this patch, users can observe Git issuing a\ntimestamp T+1 before it issues timestamp T, because Git\nsometimes uses time(NULL) or time(&t) and sometimes uses\nhigher-res methods like gettimeofday.  Although strictly\nspeaking users should tolerate this behavuior because a\nsuperuser can always change the clock back, this is a\nquality of implementation issue and users naturally expect\nGit to issue timestamps in increasing order unless the\nsuperuser has fiddled with the system clock.\n\nThis patch always uses gettimeofday(...) instead of time(...),\nand I have verified that the resulting .o files never refer\nto the name 'time'.  A trickier patch would change only\nthose calls for which timestamp monotonicity is user-visible.\nSuch a patch would require more expertise about Git internals,\nthough, and would be harder to maintain later.\n\nAnother possibility would be to change Git's documentation\nto warn users that Git does not always issue timestamps in\nincreasing order.  However, Git users would likely be either\ndismayed by this possibility, or confused by the level of\ndetail that any such documentation would require.\n\nYet another possibility would be to fix the Linux kernel so\nthat the time syscall is consistent with the other timestamp\nsyscalls.  I suppose this has not been done due to\nperformance implications.  (Git's use of timestamps is rare\nenough that performance is not a significant consideration\nfor git.)  However, this wouldn't fix Git's problem on older\nLinux kernels, or on AIX or MS-Windows.\n\nSigned-off-by: Paul Eggert <eggert@cs.ucla.edu>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This one is closer to v1 than my \"how about\" illustration, in\n   that we use gettimeofday() everywhere, so I kept the authorship\n   ident and timestamp from the original.\n\n git-compat-util.h | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4f0028ce60..d6fbe9311d 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -339,6 +339,25 @@ static inline const char *precompose_string_if_needed(const char *in)\n int compat_mkdir_wo_trailing_slash(const char*, mode_t);\n #endif\n \n+#ifdef time\n+#undef time\n+#endif\n+static inline time_t git_time(time_t *tloc)\n+{\n+\tstruct timeval tv;\n+\n+\t/*\n+\t * Avoid time(NULL), which can disagree with gettimeofday(2)\n+\t * and filesystem timestamps.\n+\t */\n+\tgettimeofday(&tv, NULL);\n+\n+\tif (tloc)\n+\t\t*tloc = tv.tv_sec;\n+\treturn tv.tv_sec;\n+}\n+#define time(x) git_time(x)\n+\n #ifdef NO_STRUCT_ITIMERVAL\n struct itimerval {\n \tstruct timeval it_interval;\n-- \n2.40.0-115-ge25cabbf6b\n\n"},{"id":"473819","messageId":"f78fd970-cce5-0a38-5ada-94ccb5bce592@cs.ucla.edu","threadId":"59413","inReplyTo":"20230320230507.3932018-1-gitster@pobox.com","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2023-03-20T23:21:40Z","receivedAt":"2023-03-20T23:22:00Z","isPatch":true,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"Thanks, this looks good. As a matter of fact it almost precisely matches \nwhat I was about to email you. The only significant difference is that \nyours has \"#define time(x) git_time(x)\" whereas mine had \"#define time \ngit_time\". Since Git never takes the address of 'time' the two macro \ndefinitions should have equivalent effects when used in Git.\n"},{"id":"473846","messageId":"xmqq4jqekreu.fsf@gitster.g","threadId":"59413","inReplyTo":"f78fd970-cce5-0a38-5ada-94ccb5bce592@cs.ucla.edu","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-21T16:20:09Z","receivedAt":"2023-03-21T16:20:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Eggert <eggert@cs.ucla.edu> writes:\n\n> Thanks, this looks good. As a matter of fact it almost precisely\n> matches what I was about to email you. The only significant difference\n> is that yours has \"#define time(x) git_time(x)\" whereas mine had\n> \"#define time git_time\". Since Git never takes the address of 'time'\n> the two macro definitions should have equivalent effects when used in\n> Git.\n\nThat is a valid concern.  Writing &time would not be caught by\ncompilers, and you would not notice such a mistake until you run \"nm\n-ug\" on the result.\n\nOn the other hand, straight token replacement will risk renaming\nvariables and structure members, and I was not sure if we have such\nuse of the identifier \"time\".  As long as people do not use \"time\"\nand \"git_time\" at the same time as such identifiers, that would not\nbe an issue (except for perhaps expecting to see them in debuggers).\nWriting \"git_time\" and \"time\" at the same time for identifiers not\nrelated to the time(2) function would not be caught by compilers,\neither, but it feels much less likely mistake we would make in the\nfuture, so let me drop (x) from the macro.\n\nThanks.\n\n\n"},{"id":"473860","messageId":"20230321174444.nuvym6ti3p3rd6hv@carbon","threadId":"59413","inReplyTo":"20230320230507.3932018-1-gitster@pobox.com","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Konstantin Khomoutov","fromEmail":"kostix@bswap.ru","sentAt":"2023-03-21T17:44:44Z","receivedAt":"2023-03-21T17:44:55Z","isPatch":true,"sender":{"key":"kostix@bswap.ru","avatar":null},"body":"On Mon, Mar 20, 2023 at 04:05:07PM -0700, Junio C Hamano wrote:\n\n[...]\n> higher-res methods like gettimeofday.  Although strictly\n> speaking users should tolerate this behavuior because a\n\nA small typo: it should probably read \"behavior\" or \"behaviour\".\n\n[...]\n\n"},{"id":"473868","messageId":"xmqq8rfqhslu.fsf@gitster.g","threadId":"59413","inReplyTo":"20230321174444.nuvym6ti3p3rd6hv@carbon","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-21T18:22:37Z","receivedAt":"2023-03-21T18:22:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Konstantin Khomoutov <kostix@bswap.ru> writes:\n\n> On Mon, Mar 20, 2023 at 04:05:07PM -0700, Junio C Hamano wrote:\n>\n> [...]\n>> higher-res methods like gettimeofday.  Although strictly\n>> speaking users should tolerate this behavuior because a\n>\n> A small typo: it should probably read \"behavior\" or \"behaviour\".\n>\n> [...]\n\nThanks.\n"},{"id":"473869","messageId":"20230321182252.GJ3119834@coredump.intra.peff.net","threadId":"59413","inReplyTo":"20230320230507.3932018-1-gitster@pobox.com","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-21T18:22:52Z","receivedAt":"2023-03-21T18:22:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2023 at 04:05:07PM -0700, Junio C Hamano wrote:\n\n> +#ifdef time\n> +#undef time\n> +#endif\n> +static inline time_t git_time(time_t *tloc)\n> +{\n> +\tstruct timeval tv;\n> +\n> +\t/*\n> +\t * Avoid time(NULL), which can disagree with gettimeofday(2)\n> +\t * and filesystem timestamps.\n> +\t */\n> +\tgettimeofday(&tv, NULL);\n> +\n> +\tif (tloc)\n> +\t\t*tloc = tv.tv_sec;\n> +\treturn tv.tv_sec;\n> +}\n> +#define time(x) git_time(x)\n\nThis looks good to me, but I wanted to mention one alternative. If we\nare declaring that time() sucks and gettimeofday() is how to do it, then\nwe could just use gettimeofday() everywhere, and add time() to banned.h\nto catch stragglers.\n\nIt has two mild advantages:\n\n  1. gettimeofday() gives the callers extra resolution if they want it\n     (though in practice I guess none of them really do)\n\n  2. It more directly describes what's going on, and we'd play fewer\n     games with macros (though we may end up with a git_gettimeofday()\n     wrapper if somebody doesn't support it; I really wonder about\n     Windows here).\n\nThe disadvantage is that it's longer to type, and that you have to\ndeclare a timeval in the caller. So maybe it's a dumb idea.\n\n-Peff\n"},{"id":"473876","messageId":"ZBoAQFCWGhwIO963@nand.local","threadId":"59413","inReplyTo":"20230321182252.GJ3119834@coredump.intra.peff.net","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-03-21T19:06:40Z","receivedAt":"2023-03-21T19:06:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 21, 2023 at 02:22:52PM -0400, Jeff King wrote:\n> The disadvantage is that it's longer to type, and that you have to\n> declare a timeval in the caller. So maybe it's a dumb idea.\n\nI don't think it's a dumb idea per-se, but I think that being able to\npass `time(NULL)` around without having to create a timeval and pass a\npointer to *it* before then giving that timeval to some other function\nis a nice advantage.\n\nSo, yeah, we probably should just avoid calling time() altogether, but\nin practice I like the solution of redefining time() to do the right\nthing and implement it by calling gettimeofday().\n\n...Which is a long way of saying that I agree with you that this\napproach looks good, and that I'd like to avoid putting time() in the\nlist of banned functions.\n\nThanks,\nTaylor\n"},{"id":"473880","messageId":"xmqqh6udhnvi.fsf@gitster.g","threadId":"59413","inReplyTo":"20230321182252.GJ3119834@coredump.intra.peff.net","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-21T20:04:49Z","receivedAt":"2023-03-21T20:05:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This looks good to me, but I wanted to mention one alternative. If we\n> are declaring that time() sucks and gettimeofday() is how to do it, then\n> we could just use gettimeofday() everywhere, and add time() to banned.h\n> to catch stragglers.\n>\n> It has two mild advantages:\n>\n>   1. gettimeofday() gives the callers extra resolution if they want it\n>      (though in practice I guess none of them really do)\n\nTrue.  Many of our data structures do not have room to store the\nextra resolution right now.\n\n>   2. It more directly describes what's going on, and we'd play fewer\n>      games with macros (though we may end up with a git_gettimeofday()\n>      wrapper if somebody doesn't support it; I really wonder about\n>      Windows here).\n\nI think they already have a compat/ function for their use in\ncompat/mingw.c so that may not be an issue.\n\n> The disadvantage is that it's longer to type, and that you have to\n> declare a timeval in the caller. So maybe it's a dumb idea.\n\nYes, you have to declare and use a timeval, but you are already\ndeclaring and using time_t in today's code, so if we were writing\nlike so from scratch, the result wouldn't have been so bad.  We just\ndo nto use time_t and use timeval instead consistently.\n\nThe patch noise to go from here to there may not be worth it,\nthough.\n\nAlso, unless we are going to actively take advantage of extra\nresolution, I am not sure if it is wise to ask for extra resolution\nin the first place.  If time(2), at least on some platforms whose\nmaintainers care, gets corrected to avoid going backwards or being\ninconsistent with filesystem timestamp in the future, converting\neverything that calls time(2) to call gettimeofday(2) would lose\ninformation---we will want to know which ones need only seconds\nresolution and convert them back when the world advances.\n\nSo, I am not quite sure I am sold on the idea of rewriting\neverything to use gettimeofday(2).\n\nThanks.\n\n"},{"id":"473935","messageId":"20230322171155.GA4296@coredump.intra.peff.net","threadId":"59413","inReplyTo":"xmqqh6udhnvi.fsf@gitster.g","subject":"Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-22T17:11:55Z","receivedAt":"2023-03-22T17:12:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 21, 2023 at 01:04:49PM -0700, Junio C Hamano wrote:\n\n> So, I am not quite sure I am sold on the idea of rewriting\n> everything to use gettimeofday(2).\n\nI'm not sure I am either. :) All your points are fair. Let's forget\nabout my suggestion.\n\n-Peff\n"}]}