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

[Junio C Hamano]

Johannes Schindelin <johannes.schinde...@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.schinde...@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() ...
    }