From e539ef5480bcb75bab83c56fa92291cb9e550099 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 5 May 2020 15:24:35 -0300 Subject: [PATCH 1/4] Fix bad rmw_time_t to nanoseconds conversion. Signed-off-by: Michel Hidalgo --- rclpy/src/rclpy_common/src/common.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/rclpy/src/rclpy_common/src/common.c b/rclpy/src/rclpy_common/src/common.c index 0d661dbc8..ecece64fc 100644 --- a/rclpy/src/rclpy_common/src/common.c +++ b/rclpy/src/rclpy_common/src/common.c @@ -140,11 +140,29 @@ rclpy_convert_to_py_names_and_types(rcl_names_and_types_t * names_and_types) return pynames_and_types; } +// TODO(hidmic): revisit after discussions in +// https://github.com/ros2/rmw/issues/210 and +// https://github.com/ros2/rmw/issues/215 have +// settled. +static +int64_t +_convert_rmw_time_to_ns(const rmw_time_t * duration) +{ + int64_t partial = RCUTILS_S_TO_NS(duration->sec); + if (partial < 0LL || (uint64_t)partial < duration->sec) { + return INT64_MAX; + } + int64_t total = partial + duration->nsec; + if (total < 0LL || total < partial || (uint64_t)total < duration->nsec) { + return INT64_MAX; + } + return total; +} + static PyObject * _convert_rmw_time_to_py_duration(const rmw_time_t * duration) { - uint64_t total_nanoseconds = RCUTILS_S_TO_NS(duration->sec) + duration->nsec; PyObject * pyduration_module = NULL; PyObject * pyduration_class = NULL; PyObject * args = NULL; @@ -163,7 +181,8 @@ _convert_rmw_time_to_py_duration(const rmw_time_t * duration) if (!args) { goto cleanup; } - kwargs = Py_BuildValue("{sK}", "nanoseconds", total_nanoseconds); + int64_t total_nanoseconds = _convert_rmw_time_to_ns(duration); + kwargs = Py_BuildValue("{sL}", "nanoseconds", total_nanoseconds); if (!kwargs) { goto cleanup; } From 0ef805b387961fad319abb2264f16ecdccc38196 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 5 May 2020 18:39:49 -0300 Subject: [PATCH 2/4] Raise on failed rmw_time_t to nanoseconds conversion. Signed-off-by: Michel Hidalgo --- rclpy/src/rclpy_common/src/common.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/rclpy/src/rclpy_common/src/common.c b/rclpy/src/rclpy_common/src/common.c index ecece64fc..63164320e 100644 --- a/rclpy/src/rclpy_common/src/common.c +++ b/rclpy/src/rclpy_common/src/common.c @@ -145,18 +145,27 @@ rclpy_convert_to_py_names_and_types(rcl_names_and_types_t * names_and_types) // https://github.com/ros2/rmw/issues/215 have // settled. static -int64_t -_convert_rmw_time_to_ns(const rmw_time_t * duration) +bool +_convert_rmw_time_to_ns(const rmw_time_t * duration, int64_t * nanoseconds) { + assert(duration != NULL); + assert(nanoseconds != NULL); int64_t partial = RCUTILS_S_TO_NS(duration->sec); if (partial < 0LL || (uint64_t)partial < duration->sec) { - return INT64_MAX; + goto fail_convert_rmw_time_to_ns; } int64_t total = partial + duration->nsec; if (total < 0LL || total < partial || (uint64_t)total < duration->nsec) { - return INT64_MAX; + goto fail_convert_rmw_time_to_ns; } - return total; + *nanoseconds = total; + return true; +fail_convert_rmw_time_to_ns: + PyErr_Format( + PyExc_RuntimeError, + "Failed to convert rmw_time_t (sec: %llu, nsec: %llu) to int64_t", + duration->sec, duration->nsec); + return false; } static @@ -181,7 +190,10 @@ _convert_rmw_time_to_py_duration(const rmw_time_t * duration) if (!args) { goto cleanup; } - int64_t total_nanoseconds = _convert_rmw_time_to_ns(duration); + int64_t total_nanoseconds = 0; + if (!_convert_rmw_time_to_ns(duration, &total_nanoseconds)) { + goto cleanup; + } kwargs = Py_BuildValue("{sL}", "nanoseconds", total_nanoseconds); if (!kwargs) { goto cleanup; From d947d01e4d3eb8717d56a4fe212afa9ab14a8683 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Wed, 6 May 2020 14:50:31 -0300 Subject: [PATCH 3/4] Please uncrustify. Signed-off-by: Michel Hidalgo --- rclpy/src/rclpy_common/src/common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rclpy/src/rclpy_common/src/common.c b/rclpy/src/rclpy_common/src/common.c index 63164320e..ed4ccaa33 100644 --- a/rclpy/src/rclpy_common/src/common.c +++ b/rclpy/src/rclpy_common/src/common.c @@ -162,9 +162,9 @@ _convert_rmw_time_to_ns(const rmw_time_t * duration, int64_t * nanoseconds) return true; fail_convert_rmw_time_to_ns: PyErr_Format( - PyExc_RuntimeError, - "Failed to convert rmw_time_t (sec: %llu, nsec: %llu) to int64_t", - duration->sec, duration->nsec); + PyExc_RuntimeError, + "Failed to convert rmw_time_t (sec: %llu, nsec: %llu) to int64_t", + duration->sec, duration->nsec); return false; } From 0981f08cdd7d8cbd08d90dbfc9b789335bf71744 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Wed, 6 May 2020 16:17:01 -0300 Subject: [PATCH 4/4] Build rclpy.Duration using seconds & nanoseconds. Signed-off-by: Michel Hidalgo --- rclpy/src/rclpy_common/src/common.c | 34 +---------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/rclpy/src/rclpy_common/src/common.c b/rclpy/src/rclpy_common/src/common.c index ed4ccaa33..ad34f0c58 100644 --- a/rclpy/src/rclpy_common/src/common.c +++ b/rclpy/src/rclpy_common/src/common.c @@ -140,34 +140,6 @@ rclpy_convert_to_py_names_and_types(rcl_names_and_types_t * names_and_types) return pynames_and_types; } -// TODO(hidmic): revisit after discussions in -// https://github.com/ros2/rmw/issues/210 and -// https://github.com/ros2/rmw/issues/215 have -// settled. -static -bool -_convert_rmw_time_to_ns(const rmw_time_t * duration, int64_t * nanoseconds) -{ - assert(duration != NULL); - assert(nanoseconds != NULL); - int64_t partial = RCUTILS_S_TO_NS(duration->sec); - if (partial < 0LL || (uint64_t)partial < duration->sec) { - goto fail_convert_rmw_time_to_ns; - } - int64_t total = partial + duration->nsec; - if (total < 0LL || total < partial || (uint64_t)total < duration->nsec) { - goto fail_convert_rmw_time_to_ns; - } - *nanoseconds = total; - return true; -fail_convert_rmw_time_to_ns: - PyErr_Format( - PyExc_RuntimeError, - "Failed to convert rmw_time_t (sec: %llu, nsec: %llu) to int64_t", - duration->sec, duration->nsec); - return false; -} - static PyObject * _convert_rmw_time_to_py_duration(const rmw_time_t * duration) @@ -190,11 +162,7 @@ _convert_rmw_time_to_py_duration(const rmw_time_t * duration) if (!args) { goto cleanup; } - int64_t total_nanoseconds = 0; - if (!_convert_rmw_time_to_ns(duration, &total_nanoseconds)) { - goto cleanup; - } - kwargs = Py_BuildValue("{sL}", "nanoseconds", total_nanoseconds); + kwargs = Py_BuildValue("{sKsK}", "seconds", duration->sec, "nanoseconds", duration->nsec); if (!kwargs) { goto cleanup; }