diff --git a/rclcpp/src/rclcpp/duration.cpp b/rclcpp/src/rclcpp/duration.cpp index 317d6b0e90..6c20757d46 100644 --- a/rclcpp/src/rclcpp/duration.cpp +++ b/rclcpp/src/rclcpp/duration.cpp @@ -67,13 +67,27 @@ Duration::operator builtin_interfaces::msg::Duration() const { builtin_interfaces::msg::Duration msg_duration; constexpr rcl_duration_value_t kDivisor = RCL_S_TO_NS(1); + constexpr int32_t max_s = std::numeric_limits::max(); + constexpr int32_t min_s = std::numeric_limits::min(); + constexpr uint32_t max_ns = std::numeric_limits::max(); const auto result = std::div(rcl_duration_.nanoseconds, kDivisor); if (result.rem >= 0) { - msg_duration.sec = static_cast(result.quot); - msg_duration.nanosec = static_cast(result.rem); + // saturate if we will overflow + if (result.quot > max_s) { + msg_duration.sec = max_s; + msg_duration.nanosec = max_ns; + } else { + msg_duration.sec = static_cast(result.quot); + msg_duration.nanosec = static_cast(result.rem); + } } else { - msg_duration.sec = static_cast(result.quot - 1); - msg_duration.nanosec = static_cast(kDivisor + result.rem); + if (result.quot <= min_s) { + msg_duration.sec = min_s; + msg_duration.nanosec = 0u; + } else { + msg_duration.sec = static_cast(result.quot - 1); + msg_duration.nanosec = static_cast(kDivisor + result.rem); + } } return msg_duration; } @@ -238,11 +252,14 @@ Duration::to_rmw_time() const throw std::runtime_error("rmw_time_t cannot be negative"); } - // reuse conversion logic from msg creation - builtin_interfaces::msg::Duration msg = *this; + // Purposefully avoid creating from builtin_interfaces::msg::Duration + // to avoid possible overflow converting from int64_t to int32_t, then back to uint64_t rmw_time_t result; - result.sec = static_cast(msg.sec); - result.nsec = static_cast(msg.nanosec); + constexpr rcl_duration_value_t kDivisor = RCL_S_TO_NS(1); + const auto div_result = std::div(rcl_duration_.nanoseconds, kDivisor); + result.sec = static_cast(div_result.quot); + result.nsec = static_cast(div_result.rem); + return result; } diff --git a/rclcpp/test/rclcpp/test_duration.cpp b/rclcpp/test/rclcpp/test_duration.cpp index 0d349d0d12..319ce06601 100644 --- a/rclcpp/test/rclcpp/test_duration.cpp +++ b/rclcpp/test/rclcpp/test_duration.cpp @@ -138,6 +138,7 @@ TEST_F(TestDuration, maximum_duration) { static const int64_t HALF_SEC_IN_NS = 500 * 1000 * 1000; static const int64_t ONE_SEC_IN_NS = 1000 * 1000 * 1000; static const int64_t ONE_AND_HALF_SEC_IN_NS = 3 * HALF_SEC_IN_NS; +static const int64_t MAX_NANOSECONDS = std::numeric_limits::max(); TEST_F(TestDuration, from_seconds) { EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration::from_seconds(0.0)); @@ -267,6 +268,34 @@ TEST_F(TestDuration, conversions) { auto chrono_duration = duration.to_chrono(); EXPECT_EQ(chrono_duration.count(), -ONE_AND_HALF_SEC_IN_NS); } + + { + auto duration = rclcpp::Duration::from_nanoseconds(MAX_NANOSECONDS); + + const auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, std::numeric_limits::max()); + EXPECT_EQ(duration_msg.nanosec, std::numeric_limits::max()); + + auto rmw_time = duration.to_rmw_time(); + EXPECT_EQ(rmw_time.sec, 9223372036u); + EXPECT_EQ(rmw_time.nsec, 854775807u); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), MAX_NANOSECONDS); + } + + { + auto duration = rclcpp::Duration::from_nanoseconds(-MAX_NANOSECONDS); + + const auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, std::numeric_limits::min()); + EXPECT_EQ(duration_msg.nanosec, 0u); + + EXPECT_THROW(duration.to_rmw_time(), std::runtime_error); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), -MAX_NANOSECONDS); + } } TEST_F(TestDuration, test_some_constructors) {