Re: [PATCH v4 7/9] Abort if the system time cannot handle one of our timestamps

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > diff --git a/date.c b/date.c
> > index 92ab31aa441..75f6335cd09 100644
> > --- a/date.c
> > +++ b/date.c
> > @@ -46,7 +46,10 @@ static time_t gm_time_t(timestamp_t time, int tz)
> >  	minutes = tz < 0 ? -tz : tz;
> >  	minutes = (minutes / 100)*60 + (minutes % 100);
> >  	minutes = tz < 0 ? -minutes : minutes;
> > -	return time + minutes * 60;
> > +
> > +	if (date_overflows(time + minutes * 60))
> > +		die("Timestamp too large for this system: %"PRItime, time);
> > +	return (time_t)time + minutes * 60;
> >  }
> 
> All the other calls to date_overflows() take a variable that holds
> timestamp_t and presumably they are checking for integer wraparound
> when the values are computed, but this one is not.

> Perhaps we want to make it a bit more careful here?  I wonder if
> something like this is a good approach:
> 
>     #define date_overflows(time) date_overflows_add(time, 0)
> 
>     int date_overflows_add(timestamp_t base, timestamp_t minutes)
>     {
> 	timestamp_t t;
> 	if (unsigned_add_overflows(base, minutes))
> 	    return 1;
> 	t = base + minutes;
> 	if ((uintmax_t) t >= TIME_MAX)
>             return 1;
> 	... what you have in date_overflows() ...
>     }