[Addons] RepositoryUpdater: fix time calculation overflow

[mglae]

Description

Using (32 bit) int for ms calculation lead to (negative) overflow when more than ~25 days ago. Using seconds std::chrono increase time span to ~68 years.

Motivation and context

When updating a LE13 test installation after a few weeks no addon updates were shown. The log looked odd:

2025-02-01 18:02:08.178 T:842     debug <general>: CRepositoryUpdater: closest next update check at 2024-12-17 7:04:17 (in 281096 s)
2025-02-01 18:02:08.178 T:842     debug <general>: CRepositoryUpdater: checking in 281096296 ms

How has this been tested?

PR included in LE13 build successfully tested on previous failing Addon33.db

What is the effect on users?

Working addon updates.

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• Student submission (PR was done for educational purposes and will be treated as such)
• None of the above (please explain below)

Checklist:

• My code follows the Code Guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the Contributing document
• I have added tests to cover my change
• All new and existing tests passed

[neo1973]

The proposed change should work, but how about a solution using std::chrono?

diff

diff --git a/xbmc/addons/RepositoryUpdater.cpp b/xbmc/addons/RepositoryUpdater.cpp
index c8c1dcbcc32..cd4a7e6b224 100644
--- a/xbmc/addons/RepositoryUpdater.cpp
+++ b/xbmc/addons/RepositoryUpdater.cpp
@@ -313,6 +313,8 @@ CDateTime CRepositoryUpdater::ClosestNextCheck() const
 
 void CRepositoryUpdater::ScheduleUpdate(UpdateScheduleType scheduleType)
 {
+  using namespace std::chrono;
+
   std::unique_lock<CCriticalSection> lock(m_criticalSection);
   m_timer.Stop(true);
 
@@ -322,14 +324,16 @@ void CRepositoryUpdater::ScheduleUpdate(UpdateScheduleType scheduleType)
   if (!m_addonMgr.HasAddons(AddonType::REPOSITORY))
     return;
 
-  int delta{1};
+  milliseconds delta{1};
   const auto nextCheck = ClosestNextCheck();
   if (nextCheck.IsValid())
   {
-    // Repos were already checked once and we know when to check next
-    delta = std::max(1, (nextCheck - CDateTime::GetCurrentDateTime()).GetSecondsTotal() * 1000);
-    CLog::Log(LOGDEBUG, "CRepositoryUpdater: closest next update check at {} (in {} s)",
-              nextCheck.GetAsLocalizedDateTime(), delta / 1000);
+    // Repos were already checked once and we know when to check next.
+    // delta must be positive and not zero (m_timer.Start() ignores 0 wait time)
+    delta = std::max<milliseconds>(
+        delta, seconds((nextCheck - CDateTime::GetCurrentDateTime()).GetSecondsTotal()));
+    CLog::Log(LOGDEBUG, "CRepositoryUpdater: closest next update check at {} (in {})",
+              nextCheck.GetAsLocalizedDateTime(), duration_cast<seconds>(delta));
   }
 
   if (scheduleType == UpdateScheduleType::Regular)
@@ -337,17 +341,12 @@ void CRepositoryUpdater::ScheduleUpdate(UpdateScheduleType scheduleType)
     // Enforce minimum hold-off time of 1 hour between regular updates - this is especially
     // important to handle all sorts of failure cases (e.g., failure to update the add-on database)
     // that would otherwise lead to an immediate new update attempt and continuous hammering of the servers.
-    delta = std::max(1 * 60 * 60 * 1'000, delta);
-  }
-  else
-  {
-    // delta must be positive and not zero (m_timer.Start() ignores 0 wait time)
-    delta = std::max(1, delta);
+    delta = std::max<milliseconds>(hours(1), delta);
   }
 
-  CLog::Log(LOGDEBUG, "CRepositoryUpdater: checking in {} ms", delta);
+  CLog::Log(LOGDEBUG, "CRepositoryUpdater: checking in {}", delta);
 
-  if (!m_timer.Start(std::chrono::milliseconds(delta)))
+  if (!m_timer.Start(delta))
     CLog::Log(LOGERROR,"CRepositoryUpdater: failed to start timer");
 }
 }

Switching to std::chrono is one of our distance goals anyway so this should be future proof.

[mglae]

The proposed change should work, but how about a solution using std::chrono?

Sure, when it does compile :)

Do you recommend updating the backport too?

[neo1973]

Thanks, looks good to me now 👍

Once merged please update the backport to match this PR.

[ksooo]

Thanks.

[mglae]

Backport is updated.