iox-eclipse-iceoryx#32 fixed issue with negative Duration values when…

… constructing from std::chrono Signed-off-by: Christian Eltzschig <[email protected]>
elBoberido · Jun 25, 2020 · 1dea2bc · 1dea2bc
1 parent a72b012
commit 1dea2bc
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 9 deletions.
diff --git a/iceoryx_utils/include/iceoryx_utils/internal/units/duration.inl b/iceoryx_utils/include/iceoryx_utils/internal/units/duration.inl
@@ -71,12 +71,12 @@ inline constexpr Duration::Duration(const struct itimerspec& value)
 }
 
 inline constexpr Duration::Duration(const std::chrono::milliseconds& value)
-    : durationInSeconds(static_cast<long double>(value.count()) / 1000.0)
+    : Duration(static_cast<long double>(value.count()) / 1000.0)
 {
 }
 
 inline constexpr Duration::Duration(const std::chrono::nanoseconds& value)
-    : durationInSeconds(static_cast<long double>(value.count()) / 1000000000.0)
+    : Duration(static_cast<long double>(value.count()) / 1000000000.0)
 {
 }
 

diff --git a/iceoryx_utils/source/units/duration.cpp b/iceoryx_utils/source/units/duration.cpp
@@ -21,10 +21,12 @@ namespace units
 {
 struct timespec Duration::timespec(const TimeSpecReference& reference) const
 {
+    constexpr int64_t NanoSecondsPerSecond{1000000000};
     if (reference == TimeSpecReference::None)
     {
-        return {this->seconds<int32_t>(),
-                static_cast<int32_t>(this->nanoSeconds<int64_t>() - this->seconds<int64_t>() * 1000000000)};
+        int64_t timeInNanoSeconds = this->nanoSeconds<int64_t>();
+        int64_t seconds = timeInNanoSeconds / NanoSecondsPerSecond;
+        return {seconds, timeInNanoSeconds - seconds * NanoSecondsPerSecond};
     }
     else
     {
@@ -41,11 +43,12 @@ struct timespec Duration::timespec(const TimeSpecReference& reference) const
         }
         else
         {
-            constexpr int64_t NanoSecondsPerSecond{1000000000};
-            int64_t remainingNanoSecondsTimeout = this->nanoSeconds<int64_t>() % NanoSecondsPerSecond;
+            int64_t timeInNanoSeconds = this->nanoSeconds<int64_t>();
+            int64_t remainingNanoSecondsTimeout = timeInNanoSeconds % NanoSecondsPerSecond;
             int64_t sumOfNanoSeconds = remainingNanoSecondsTimeout + referenceTime.tv_nsec;
-            int64_t seconds = this->seconds<int64_t>() + referenceTime.tv_sec + sumOfNanoSeconds / NanoSecondsPerSecond;
-            int64_t nanoSeconds = sumOfNanoSeconds - (sumOfNanoSeconds / NanoSecondsPerSecond) * NanoSecondsPerSecond;
+            int64_t additionalSeconds = sumOfNanoSeconds / NanoSecondsPerSecond;
+            int64_t seconds = timeInNanoSeconds / NanoSecondsPerSecond + referenceTime.tv_sec + additionalSeconds;
+            int64_t nanoSeconds = sumOfNanoSeconds - additionalSeconds * NanoSecondsPerSecond;
 
             return {seconds, nanoSeconds};
         }

diff --git a/iceoryx_utils/test/moduletests/test_unit_duration.cpp b/iceoryx_utils/test/moduletests/test_unit_duration.cpp
@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "test.hpp"
 #include "iceoryx_utils/internal/units/duration.hpp"
+#include "test.hpp"
 
 using namespace ::testing;
 using namespace iox::units;
@@ -221,3 +221,15 @@ TEST(Duration_test, constructFromTimeval)
     Duration result(value);
     EXPECT_EQ(result.microSeconds<uint64_t>(), 42 + 1000000 * 1337);
 }
+
+TEST(Duration_test, isZeroWhenConstructedFromNegativeChronoMilliSeconds)
+{
+    Duration result(std::chrono::milliseconds(-1));
+    EXPECT_EQ(result.milliSeconds<uint64_t>(), 0u);
+}
+
+TEST(Duration_test, isZeroWhenConstructedFromNegativeChronoNanoSeconds)
+{
+    Duration result(std::chrono::nanoseconds(-1));
+    EXPECT_EQ(result.nanoSeconds<uint64_t>(), 0u);
+}