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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2023, 20:04 UTC
Message-ID: <xmqqh6udhnvi.fsf@gitster.g>
In-Reply-To: <20230321182252.GJ3119834@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

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

True. Many of our data structures do not have room to store the extra resolution right now.

>   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).

I think they already have a compat/ function for their use in compat/mingw.c so that may not be an issue.

> 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.

Yes, you have to declare and use a timeval, but you are already declaring and using time_t in today's code, so if we were writing like so from scratch, the result wouldn't have been so bad. We just do nto use time_t and use timeval instead consistently.

The patch noise to go from here to there may not be worth it, though.

Also, unless we are going to actively take advantage of extra resolution, I am not sure if it is wise to ask for extra resolution in the first place. If time(2), at least on some platforms whose maintainers care, gets corrected to avoid going backwards or being inconsistent with filesystem timestamp in the future, converting everything that calls time(2) to call gettimeofday(2) would lose information---we will want to know which ones need only seconds resolution and convert them back when the world advances.

So, I am not quite sure I am sold on the idea of rewriting everything to use gettimeofday(2).

Thanks.