From 3a28d78089289794fda86cdbd275fc4756c6c6aa Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 26 Jan 2023 17:09:12 -0500 Subject: Improve TimestampDifferenceMilliseconds to cope with overflow sanely. We'd like to use TimestampDifferenceMilliseconds with the stop_time possibly being TIMESTAMP_INFINITY, but up to now it's disclaimed responsibility for overflow cases. Define it to clamp its output to the range [0, INT_MAX], handling overflow correctly. (INT_MAX rather than LONG_MAX seems appropriate, because the function is already described as being intended for calculating wait times for WaitLatch et al, and that infrastructure only handles waits up to INT_MAX. Also, this choice gets rid of cross-platform behavioral differences.) Having done that, we can replace some ad-hoc code in walreceiver.c with a simple call to TimestampDifferenceMilliseconds. While at it, fix some buglets in existing callers of TimestampDifferenceMilliseconds: basebackup_copy.c had not read the memo about TimestampDifferenceMilliseconds never returning a negative value, and postmaster.c had not read the memo about Min() and Max() being macros with multiple-evaluation hazards. Neither of these quite seem worth back-patching. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/3126727.1674759248@sss.pgh.pa.us --- src/backend/utils/adt/timestamp.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) (limited to 'src/backend/utils/adt/timestamp.c') diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 928c3308973..47e059a409b 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -1690,26 +1690,31 @@ TimestampDifference(TimestampTz start_time, TimestampTz stop_time, * * This is typically used to calculate a wait timeout for WaitLatch() * or a related function. The choice of "long" as the result type - * is to harmonize with that. It is caller's responsibility that the - * input timestamps not be so far apart as to risk overflow of "long" - * (which'd happen at about 25 days on machines with 32-bit "long"). - * - * Both inputs must be ordinary finite timestamps (in current usage, - * they'll be results from GetCurrentTimestamp()). + * is to harmonize with that; furthermore, we clamp the result to at most + * INT_MAX milliseconds, because that's all that WaitLatch() allows. * * We expect start_time <= stop_time. If not, we return zero, * since then we're already past the previously determined stop_time. * + * Subtracting finite and infinite timestamps works correctly, returning + * zero or INT_MAX as appropriate. + * * Note we round up any fractional millisecond, since waiting for just * less than the intended timeout is undesirable. */ long TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time) { - TimestampTz diff = stop_time - start_time; + TimestampTz diff; - if (diff <= 0) + /* Deal with zero or negative elapsed time quickly. */ + if (start_time >= stop_time) return 0; + /* To not fail with timestamp infinities, we must detect overflow. */ + if (pg_sub_s64_overflow(stop_time, start_time, &diff)) + return (long) INT_MAX; + if (diff >= (INT_MAX * INT64CONST(1000) - 999)) + return (long) INT_MAX; else return (long) ((diff + 999) / 1000); } -- cgit v1.2.3