Applied suggestions

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Author: EugenioCollado

cpp_utils/include/cpp_utils/time/time_utils.hpp

@@ -30,9 +30,13 @@ namespace utils {
 using Duration_ms = uint32_t;
 
 /**
- * Type used to represent time points
+ * Type used to fix the clock to the system clock
  */
 using Timeclock = std::chrono::system_clock;
+
+/**
+ * Type used to represent time points
+ */
 using Timestamp = std::chrono::time_point<Timeclock>;
 
 /**

cpp_utils/src/cpp/time/time_utils.cpp

@@ -41,20 +41,12 @@ Timestamp now() noexcept
 
 Timestamp the_end_of_time() noexcept
 {
-    #if _EPROSIMA_WINDOWS_PLATFORM // In Windows std::gmtime does not support negative values nor values greater than 2^32
-    return Timeclock::from_time_t(std::numeric_limits<long>::max());
-    #else
     return Timestamp::max();
-    #endif // if _EPROSIMA_WINDOWS_PLATFORM
 }
 
 Timestamp the_beginning_of_time() noexcept
 {
-    #if _EPROSIMA_WINDOWS_PLATFORM // In Windows std::gmtime does not support negative values nor values greater than 2^32
-    return Timeclock::from_time_t(0);
-    #else
     return Timestamp::min();
-    #endif // if _EPROSIMA_WINDOWS_PLATFORM
 }
 
 Timestamp date_to_timestamp(
@@ -106,8 +98,14 @@ std::string timestamp_to_string(
     time_t duration_seconds = std::chrono::duration_cast<std::chrono::seconds>(timestamp.time_since_epoch()).count();
 
     #if _EPROSIMA_WINDOWS_PLATFORM // In Windows std::gmtime does not support negative values nor values greater than 2^32
-    time_t max_value = std::numeric_limits<long>::max();
-    duration_seconds = std::max((time_t) 0, std::min(max_value, duration_seconds));
+    time_t max_value = std::numeric_limits<int32_t>::max();
+    if (0 > duration_seconds || duration_seconds > max_value)
+    {
+        EPROSIMA_LOG_WARNING(TIME_UTILS,
+                "Timestamp value: " << duration_seconds << " is out of range for Windows, clamping to 0 and " <<
+                max_value);
+        duration_seconds = std::max((time_t) 0, std::min(max_value, duration_seconds));
+    }
     #endif // if _EPROSIMA_WINDOWS_PLATFORM
 
     std::tm* tm = nullptr;

cpp_utils/test/unittest/time/time_utils_test.cpp

@@ -31,11 +31,12 @@ using namespace eprosima::utils;
  * - now
  * - now with alternative format
  * - old time
+ * - date before 1970
  * - the beginning of time
  * - future time
+ * - date after 2038
  * - the end of time
  * - some time today
- * - wrong date and time
  */
 TEST(time_utils_test, timestamp_to_string_to_timestamp)
 {
@@ -115,6 +116,39 @@ TEST(time_utils_test, timestamp_to_string_to_timestamp)
         ASSERT_EQ(timestamp_to_string(old_time_from_str), expected_string_os.str());
     }
 
+    // date before 1970
+    {
+        Timestamp old_time = date_to_timestamp(1959u, 7u, 20u, 6u, 39u, 42u);
+        std::string old_time_str = timestamp_to_string(old_time);
+
+        std::ostringstream expected_string_os;
+        #if _EPROSIMA_WINDOWS_PLATFORM
+        expected_string_os
+            << 1970
+            << "-" << "01"
+            << "-" << "01"
+            << "_" << "00"
+            << "-" << "00"
+            << "-" << "00";
+        #else
+        expected_string_os
+            << 1959
+            << "-" << "07"
+            << "-" << 20
+            << "_" << "06"
+            << "-" << 39
+            << "-" << 42;
+        #endif  // _EPROSIMA_WINDOWS_PLATFORM
+
+        // Test timestamp_to_string
+        ASSERT_EQ(old_time_str, expected_string_os.str());
+
+        // Test string_to_timestamp
+        Timestamp old_time_from_str = string_to_timestamp(old_time_str);
+        // NOTE: cannot directly compare timestamps because some precision is lost during ts->str conversion
+        ASSERT_EQ(timestamp_to_string(old_time_from_str), expected_string_os.str());
+    }
+
     // the begininng of time
     {
         Timestamp beginning_time = the_beginning_of_time();
@@ -172,6 +206,39 @@ TEST(time_utils_test, timestamp_to_string_to_timestamp)
         ASSERT_EQ(timestamp_to_string(future_time_from_str), expected_string_os.str());
     }
 
+    // date after 2038
+    {
+        Timestamp future_time = date_to_timestamp(2049u, 5u, 22u);
+        std::string future_time_str = timestamp_to_string(future_time);
+
+        std::ostringstream expected_string_os;
+        #if _EPROSIMA_WINDOWS_PLATFORM
+        expected_string_os
+            << 2038
+            << "-" << "01"
+            << "-" << 19
+            << "_" << "03"
+            << "-" << "14"
+            << "-" << "07";
+        #else
+        expected_string_os
+            << 2049
+            << "-" << "05"
+            << "-" << 22
+            << "_" << "00"
+            << "-" << "00"
+            << "-" << "00";
+        #endif  // _EPROSIMA_WINDOWS_PLATFORM
+
+        // Test timestamp_to_string
+        ASSERT_EQ(future_time_str, expected_string_os.str());
+
+        // Test string_to_timestamp
+        Timestamp future_time_from_str = string_to_timestamp(future_time_str);
+        // NOTE: cannot directly compare timestamps because some precision is lost during ts->str conversion
+        ASSERT_EQ(timestamp_to_string(future_time_from_str), expected_string_os.str());
+    }
+
     // the end of time
     {
         Timestamp end_time = the_end_of_time();