Break out overflow-calculations into semantically-named inlines

Adding days to seconds or millis, or seconds to millis, involves scaling one and adding the other, so has to check for overflow. The std::integral_constant boilerplate and the complications of calling mul_overflow() and add_overflow() rather hid what was going on, so package them in inlines so that their calls are more intelligible. Change-Id: I1e90de8fcb81eb84920868c7e4bd217ee353fc54 Reviewed-by: Thiago Macieira <[email protected]> Reviewed-by: Mårten Nordheim <[email protected]>
zoldalis · Aug 19, 2022 · b9baa42 · b9baa42
1 parent acb2faf
commit b9baa42
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 22 deletions.
diff --git a/src/corelib/time/qdatetime.cpp b/src/corelib/time/qdatetime.cpp
@@ -2451,6 +2451,14 @@ static QTime msecsToTime(qint64 msecs)
     return QTime::fromMSecsSinceStartOfDay(QRoundingDown::qMod(msecs, MSECS_PER_DAY));
 }
 
+// True if combining days with millis overflows; otherwise, stores result in *sumMillis
+// The inputs should not have opposite signs.
+static inline bool daysAndMillisOverflow(qint64 days, qint64 millisInDay, qint64 *sumMillis)
+{
+    return mul_overflow(days, std::integral_constant<qint64, MSECS_PER_DAY>(), sumMillis)
+        || add_overflow(*sumMillis, millisInDay, sumMillis);
+}
+
 // Converts a date/time value into msecs
 static qint64 timeToMSecs(QDate date, QTime time)
 {
@@ -2460,8 +2468,7 @@ static qint64 timeToMSecs(QDate date, QTime time)
         ++days;
         dayms -= MSECS_PER_DAY;
     }
-    if (mul_overflow(days, std::integral_constant<qint64, MSECS_PER_DAY>(), &msecs)
-        || add_overflow(msecs, dayms, &msecs)) {
+    if (daysAndMillisOverflow(days, dayms, &msecs)) {
         using Bound = std::numeric_limits<qint64>;
         return days < 0 ? Bound::min() : Bound::max();
     }
@@ -2584,13 +2591,11 @@ static auto millisToWithinRange(qint64 millis)
         qint64 shifted = 0;
         bool good = false;
     } result;
-    qint64 jd = msecsToJulianDay(millis), fakeJd, diffMillis;
+    qint64 jd = msecsToJulianDay(millis), fakeJd;
     auto ymd = QGregorianCalendar::partsFromJulian(jd);
     result.good = QGregorianCalendar::julianFromParts(systemTimeYearMatching(ymd.year),
                                                       ymd.month, ymd.day, &fakeJd)
-        && !mul_overflow(fakeJd - jd, std::integral_constant<qint64, MSECS_PER_DAY>(),
-                         &diffMillis)
-        && !add_overflow(diffMillis, millis, &result.shifted);
+        && !daysAndMillisOverflow(fakeJd - jd, millis, &result.shifted);
     return result;
 }
 
@@ -2951,8 +2956,7 @@ static void setDateTime(QDateTimeData &d, QDate date, QTime time)
 
     // Check in representable range:
     qint64 msecs = 0;
-    if (mul_overflow(days, std::integral_constant<qint64, MSECS_PER_DAY>(), &msecs)
-        || add_overflow(msecs, qint64(ds), &msecs)) {
+    if (daysAndMillisOverflow(days, qint64(ds), &msecs)) {
         newStatus = QDateTimePrivate::StatusFlags{};
         msecs = 0;
     }

diff --git a/src/corelib/time/qlocaltime.cpp b/src/corelib/time/qlocaltime.cpp
@@ -283,6 +283,24 @@ int getUtcOffset(qint64 atMSecsSinceEpoch)
 }
 #endif // QT_BOOTSTRAPPED
 
+#define IC(N) std::integral_constant<qint64, N>()
+
+// True if combining day and seconds overflows qint64; otherwise, sets *epochSeconds
+inline bool daysAndSecondsOverflow(qint64 julianDay, qint64 daySeconds, qint64 *epochSeconds)
+{
+    return mul_overflow(julianDay - JULIAN_DAY_FOR_EPOCH, IC(SECS_PER_DAY), epochSeconds)
+        || add_overflow(*epochSeconds, daySeconds, epochSeconds);
+}
+
+// True if combining seconds and millis overflows; otherwise sets *epochMillis
+inline bool secondsAndMillisOverflow(qint64 epochSeconds, qint64 millis, qint64 *epochMillis)
+{
+    return mul_overflow(epochSeconds, IC(MSECS_PER_SEC), epochMillis)
+        || add_overflow(*epochMillis, millis, epochMillis);
+}
+
+#undef IC
+
 // Calls the platform variant of localtime() for the given utcMillis, and
 // returns the local milliseconds, offset from UTC and DST status.
 QDateTimePrivate::ZoneState utcToLocal(qint64 utcMillis)
@@ -306,13 +324,8 @@ QDateTimePrivate::ZoneState utcToLocal(qint64 utcMillis)
     const qint64 daySeconds = tmSecsWithinDay(local);
     Q_ASSERT(0 <= daySeconds && daySeconds < SECS_PER_DAY);
     qint64 localSeconds, localMillis;
-    if (Q_UNLIKELY(
-            mul_overflow(jd - JULIAN_DAY_FOR_EPOCH, std::integral_constant<qint64, SECS_PER_DAY>(),
-                         &localSeconds)
-            || add_overflow(localSeconds, daySeconds, &localSeconds)
-            || mul_overflow(localSeconds, std::integral_constant<qint64, MSECS_PER_SEC>(),
-                            &localMillis)
-            || add_overflow(localMillis, qint64(msec), &localMillis))) {
+    if (Q_UNLIKELY(daysAndSecondsOverflow(jd, daySeconds, &localSeconds)
+                   || secondsAndMillisOverflow(localSeconds, qint64(msec), &localMillis))) {
         return {utcMillis};
     }
     const auto dst
@@ -362,21 +375,17 @@ QDateTimePrivate::ZoneState mapLocalTime(qint64 local, QDateTimePrivate::Dayligh
         daySecs -= SECS_PER_DAY;
     }
     qint64 localSecs;
-    if (Q_UNLIKELY(mul_overflow(jd - JULIAN_DAY_FOR_EPOCH,
-                                std::integral_constant<qint64, SECS_PER_DAY>(), &localSecs)
-                   || add_overflow(localSecs, daySecs, &localSecs))) {
+    if (Q_UNLIKELY(daysAndSecondsOverflow(jd, daySecs, &localSecs)))
         return {local, offset, dst, false};
-    }
+
     offset = localSecs - utcSecs;
 
     if (localSecs < 0 && millis > 0) {
         ++localSecs;
         millis -= MSECS_PER_SEC;
     }
     qint64 revised;
-    const bool overflow =
-        mul_overflow(localSecs, std::integral_constant<qint64, MSECS_PER_SEC>(), &revised)
-        || add_overflow(revised, millis, &revised);
+    const bool overflow = secondsAndMillisOverflow(localSecs, millis, &revised);
     return {overflow ? local : revised, offset, dst, !overflow};
 }