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

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> We are about to switch to a new data type for time stamps that is
> definitely not smaller or equal, but larger or equal to time_t.
>
> So before using the system functions to process or format timestamps,
> let's make extra certain that they can handle what we feed them.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  date.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> 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() ...
    }