Re: [PATCH v3 0/8] Introduce timestamp_t for timestamps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> writes:

> Note: while the `time_t` data type exists and is meant to be used for
> timestamps, on 32-bit Linux it is *still* 32-bit. An earlier iteration
> used `time_t` for that reason, but it came with a few serious downsides:
> as `time_t` can be signed (and indeed, on Windows it is an int64_t),
> Git's expectation that 0 is the minimal value does no longer hold true,
> introducing its own set of interesting challenges. Besides, if we *can*
> handle far in the future timestamps (except for formatting them using
> the system libraries), it is more consistent to do so.

I somehow had an impression that the list consensus during the
discussion on an earlier round of this series was that time_t is
more appropriate, as platforms with time_t with inadequent range
will be updated before it gets too late at around 2038 (or they will
die off).  After all, at some point we need to interact with the
platform functions that expect time_t as their interface and they do
not take our own timestamp_t without casting.

But that is provided if not introducing timestamp_t and using time_t
results in a simpler code.  I do not know if that is the case.

For timestamps in the distant past, even though time_t could be
unsigned, I do not think anybody came up with a concrete example of
a platform with such a problem during the previous discussions,
while I do recall people wanting to use Git to store historical
documents with timestamps before 1970.  We do expect 0 can be used
as a sentinel, which needs to be updated once we seriously start
supporting such use cases.  I think that (i.e. should the timestamp
be signed?) is more or less unrelated to the focus of the discussion
that stemed from this topic, which was "ulong that is often 32-bit
does not necessarily fit the notion of time on a platform, which is
time_t, and we need to widen it", which all involved in the discussion
agreed.

In any case, when merged to 'pu', this had a slight conflict with
topics in flight in builtin/name-rev.c and I think I resolved it
correctly, but please double check.  I will have to revamp the
resolution rerere remembered when the next version of this series
starts using time_t instead of timestamp_t anyway, but it is better
to always get conflict resolution right.  That is what maintainers
do and what they are for ;-)

Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]