addressed potential overflow, and extended timer test to check if ordering is working correctly

lib/include/srslte/common/timers.h

@@ -30,6 +30,7 @@
 #define SRSLTE_TIMERS_H
 
 #include <functional>
+#include <limits>
 #include <queue>
 #include <stdint.h>
 #include <stdio.h>
@@ -185,25 +186,28 @@ class timers2
     bool     is_running() const { return active and running and timeout > 0; }
     bool     is_expired() const { return active and not running and timeout > 0; }
 
-    void set(uint32_t duration_, std::function<void(int)> callback_)
+    bool set(uint32_t duration_)
     {
+      if (duration_ > std::numeric_limits<uint32_t>::max() / 4) {
+        ERROR("Error: timer durations above %u are not supported\n", std::numeric_limits<uint32_t>::max() / 4);
+        return false;
+      }
       if (not active) {
         ERROR("Error: setting inactive timer id=%d\n", id());
-        return;
+        return false;
       }
-      callback = std::move(callback_);
       duration = duration_;
       running  = false; // invalidates any on-going run
+      return true;
     }
 
-    void set(uint32_t duration_)
+    bool set(uint32_t duration_, std::function<void(int)> callback_)
     {
-      if (not active) {
+      if (set(duration_)) {
-        ERROR("Error: setting inactive timer id=%d\n", id());
+        callback = std::move(callback_);
-        return;
+        return true;
       }
-      duration = duration_;
+      return false;
-      running  = false; // invalidates any on-going run
     }
 
     void run()
@@ -333,7 +337,15 @@ private:
       timeout(timeout_)
     {
     }
-    bool operator<(const timer_run& other) const { return timeout > other.timeout; }
+    bool operator<(const timer_run& other) const
+    {
+      // returns true, if greater
+      uint32_t lim = std::numeric_limits<uint32_t>::max();
+      if (timeout > other.timeout) {
+        return (timeout - other.timeout) < lim / 2;
+      }
+      return (other.timeout - timeout) > lim / 2;
+    }
   };
 
   std::vector<timer_impl>        timer_list;

lib/test/common/timer_test.cc

@@ -88,6 +88,35 @@ int timers2_test()
   TESTASSERT(not t.is_running())
   TESTASSERT(callback_called)
 
+  // TEST: ordering of timers is respected
+  timers2::unique_timer t3 = timers.get_unique_timer();
+  TESTASSERT(t3.id() == 2)
+  int  first_id = -1, last_id = -1;
+  auto callback = [&first_id, &last_id](int id) {
+    if (first_id < 0) {
+      printf("First timer id=%d\n", id);
+      first_id = id;
+    }
+    last_id = id;
+  };
+  t.set(4, callback);
+  t2.set(2, callback);
+  t3.set(6, callback);
+  t.run();
+  t2.run();
+  t3.run();
+  for (uint32_t i = 0; i < 6; ++i) {
+    timers.step_all();
+    TESTASSERT(i >= 4 or t.is_running())
+    TESTASSERT(i >= 2 or t2.is_running())
+    TESTASSERT(t3.is_running())
+  }
+  timers.step_all();
+  TESTASSERT(t.is_expired() and t2.is_expired() and t3.is_expired())
+  TESTASSERT(first_id == 1)
+  printf("Last timer id=%d\n", last_id);
+  TESTASSERT(last_id == 2)
+
   printf("Success\n");
 
   return SRSLTE_SUCCESS;