Subject: Re: [PATCH] name-rev: avoid cutoff timestamp underflow

From: SZEDER Gábor <[email-masked]>
Date: Tue, 24 Sep 2019 09:21:59 +0200

On Mon, Sep 23, 2019 at 09:16:26PM +0200, Johannes Sixt wrote:
> Am 23.09.19 um 10:37 schrieb SZEDER Gábor:
> > On Sun, Sep 22, 2019 at 11:01:26PM +0200, Johannes Sixt wrote:
> >> Huh? For signed cutoff and positive CUTOFF_DATE_SLOP,
> >> cutoff - CUTOFF_DATE_SLOP < cutoff is ALWAYS true. Signed interger
> >> underflow is undefined behavior and signed integer arithmetic does not
> >> wrap around!
> >>
> >> IOW, the new condition makes only sense today, because cutoff is an
> >> unsigned type, but breaks down should we switch to a signed type.
> > 
> > Yeah, that's what I meant with worrying about signed underflow in the
> > commit message.  As long as the cutoff is at least a day later than
> > the minimum value of our future signed 'timestamp_t', the condition
> > does the right thing.  And considering that oldest time a signed 64
> > bit timestamp can represent far exceeds the age of the universe, and
> > the oldest value of even a signed 32 bit timestamp is almost half the
> > age of the Earth, I wasn't too worried.
> 
> Note that commits and timestamps can be forged easily. I'm not worried
> that Git does not work "correctly" with forged timestamps (as long as it
> is not exploitable); but when it is simple to make foolproof, we should
> do it.
> 
> BTW, 32-bit timestamps cover just ~135 years (not half the age of
> Earth).

Gah, forgot the division with seconds/year when calculating the range
of the 32bit timestamp.

> That's too little for people who want to store historic
> documents in a Git repository.

Indeed, but 'timestamp_t' is defined as 'uintmax_t', and we have the
Makefile knob 'NO_UINTMAX_T', in which case 'uintmax_t' is defined as
'uint32_t'...