Re: [PATCH v2] git-compat-util: use gettimeofday(2) for time(2)

From: Jeff King <peff@peff.net>
Date: Mar 21, 2023, 18:22 UTC
Message-ID: <20230321182252.GJ3119834@coredump.intra.peff.net>
In-Reply-To: <20230320230507.3932018-1-gitster@pobox.com>

On Mon, Mar 20, 2023 at 04:05:07PM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> +#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)

This looks good to me, but I wanted to mention one alternative. If we are declaring that time() sucks and gettimeofday() is how to do it, then we could just use gettimeofday() everywhere, and add time() to banned.h to catch stragglers.

It has two mild advantages:

  1. gettimeofday() gives the callers extra resolution if they want it
     (though in practice I guess none of them really do)

  2. It more directly describes what's going on, and we'd play fewer
     games with macros (though we may end up with a git_gettimeofday()
     wrapper if somebody doesn't support it; I really wonder about
     Windows here).

The disadvantage is that it's longer to type, and that you have to declare a timeval in the caller. So maybe it's a dumb idea.

-Peff