Add additional null check for timer argument (ros2#973)

Signed-off-by: Noy <noy@xtend.me>
ApexAI · Mar 30, 2022 · 8ae0c9d · 8ae0c9d
1 parent 8dd9905
commit 8ae0c9d
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 11 deletions.
diff --git a/rcl/include/rcl/timer.h b/rcl/include/rcl/timer.h
@@ -225,7 +225,7 @@ rcl_timer_fini(rcl_timer_t * timer);
  * \param[inout] timer the handle to the timer to call
  * \return #RCL_RET_OK if the timer was called successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_TIMER_CANCELED if the timer has been canceled, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
@@ -250,10 +250,9 @@ rcl_timer_call(rcl_timer_t * timer);
  *
  * \param[in] timer the handle to the timer which is being queried
  * \param[out] clock the rcl_clock_t * in which the clock is stored
- * \return #RCL_RET_OK if the period was retrieved successfully, or
+ * \return #RCL_RET_OK if the clock was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
- * \return #RCL_RET_ERROR an unspecified error occur.
+ * \return #RCL_RET_TIMER_INVALID if the timer is invalid.
  */
 RCL_PUBLIC
 RCL_WARN_UNUSED
@@ -282,7 +281,7 @@ rcl_timer_clock(rcl_timer_t * timer, rcl_clock_t ** clock);
  * \param[out] is_ready the bool used to store the result of the calculation
  * \return #RCL_RET_OK if the last call time was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
 RCL_PUBLIC
@@ -317,7 +316,7 @@ rcl_timer_is_ready(const rcl_timer_t * timer, bool * is_ready);
  * \param[out] time_until_next_call the output variable for the result
  * \return #RCL_RET_OK if the timer until next call was successfully calculated, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_TIMER_CANCELED if the timer is canceled, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
@@ -350,7 +349,7 @@ rcl_timer_get_time_until_next_call(const rcl_timer_t * timer, int64_t * time_unt
  * \param[out] time_since_last_call the struct in which the time is stored
  * \return #RCL_RET_OK if the last call time was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
 RCL_PUBLIC
@@ -377,6 +376,7 @@ rcl_timer_get_time_since_last_call(const rcl_timer_t * timer, int64_t * time_sin
  * \param[out] period the int64_t in which the period is stored
  * \return #RCL_RET_OK if the period was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
 RCL_PUBLIC
@@ -407,6 +407,7 @@ rcl_timer_get_period(const rcl_timer_t * timer, int64_t * period);
  * \param[out] old_period the int64_t in which the previous period is stored
  * \return #RCL_RET_OK if the period was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
 RCL_PUBLIC
@@ -482,10 +483,9 @@ rcl_timer_exchange_callback(rcl_timer_t * timer, const rcl_timer_callback_t new_
  * <i>[1] if `atomic_is_lock_free()` returns true for `atomic_int_least64_t`</i>
  *
  * \param[inout] timer the timer to be canceled
- * \return #RCL_RET_OK if the last call time was retrieved successfully, or
+ * \return #RCL_RET_OK if the timer was canceled successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
- * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
- * \return #RCL_RET_ERROR an unspecified error occur.
+ * \return #RCL_RET_TIMER_INVALID if the timer is invalid.
  */
 RCL_PUBLIC
 RCL_WARN_UNUSED
@@ -513,6 +513,7 @@ rcl_timer_cancel(rcl_timer_t * timer);
  * \param[out] is_canceled storage for the is canceled bool
  * \return #RCL_RET_OK if the last call time was retrieved successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
+ * \return #RCL_RET_TIMER_INVALID if the timer->impl is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.
  */
 RCL_PUBLIC
@@ -536,7 +537,7 @@ rcl_timer_is_canceled(const rcl_timer_t * timer, bool * is_canceled);
  * <i>[1] if `atomic_is_lock_free()` returns true for `atomic_int_least64_t`</i>
  *
  * \param[inout] timer the timer to be reset
- * \return #RCL_RET_OK if the last call time was retrieved successfully, or
+ * \return #RCL_RET_OK if the timer was reset successfully, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
  * \return #RCL_RET_TIMER_INVALID if the timer is invalid, or
  * \return #RCL_RET_ERROR an unspecified error occur.

diff --git a/rcl/src/rcl/timer.c b/rcl/src/rcl/timer.c
@@ -244,6 +244,7 @@ rcl_timer_call(rcl_timer_t * timer)
 {
   RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Calling timer");
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   if (rcutils_atomic_load_bool(&timer->impl->canceled)) {
     RCL_SET_ERROR_MSG("timer is canceled");
     return RCL_RET_TIMER_CANCELED;
@@ -294,6 +295,7 @@ rcl_ret_t
 rcl_timer_is_ready(const rcl_timer_t * timer, bool * is_ready)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(is_ready, RCL_RET_INVALID_ARGUMENT);
   int64_t time_until_next_call;
   rcl_ret_t ret = rcl_timer_get_time_until_next_call(timer, &time_until_next_call);
@@ -311,6 +313,7 @@ rcl_ret_t
 rcl_timer_get_time_until_next_call(const rcl_timer_t * timer, int64_t * time_until_next_call)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(time_until_next_call, RCL_RET_INVALID_ARGUMENT);
   if (rcutils_atomic_load_bool(&timer->impl->canceled)) {
     return RCL_RET_TIMER_CANCELED;
@@ -331,6 +334,7 @@ rcl_timer_get_time_since_last_call(
   rcl_time_point_value_t * time_since_last_call)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(time_since_last_call, RCL_RET_INVALID_ARGUMENT);
   rcl_time_point_value_t now;
   rcl_ret_t ret = rcl_clock_get_now(timer->impl->clock, &now);
@@ -346,6 +350,7 @@ rcl_ret_t
 rcl_timer_get_period(const rcl_timer_t * timer, int64_t * period)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(period, RCL_RET_INVALID_ARGUMENT);
   *period = rcutils_atomic_load_uint64_t(&timer->impl->period);
   return RCL_RET_OK;
@@ -357,6 +362,7 @@ rcl_timer_exchange_period(const rcl_timer_t * timer, int64_t new_period, int64_t
   RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
 
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(old_period, RCL_RET_INVALID_ARGUMENT);
   *old_period = rcutils_atomic_exchange_uint64_t(&timer->impl->period, new_period);
   RCUTILS_LOG_DEBUG_NAMED(
@@ -400,6 +406,7 @@ rcl_ret_t
 rcl_timer_is_canceled(const rcl_timer_t * timer, bool * is_canceled)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(timer->impl, RCL_RET_TIMER_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(is_canceled, RCL_RET_INVALID_ARGUMENT);
   *is_canceled = rcutils_atomic_load_bool(&timer->impl->canceled);
   return RCL_RET_OK;