[PATCH 1/3] _TOD_Validate(): Fix incorrect return code

Sebastian Huber sebastian.huber at embedded-brains.de
Tue May 11 15:38:13 UTC 2021


From: Frank Kühndel <frank.kuehndel at embedded-brains.de>

This patch fixes bug #4403. Directives

* rtems_timer_fire_when()
* rtems_timer_server_fire_when()
* rtems_task_wake_when()

are documented to return RTEMS_INVALID_ADDRESS when their time-of-day
argument is NULL. But actually they return RTEMS_INVALID_CLOCK. To fix
the issue this patch changes _TOD_Validate() to return a
status code instead of just true/false.

Close #4403
---
 bsps/arm/altera-cyclone-v/rtc/rtc.c    | 10 ++++----
 bsps/shared/dev/rtc/rtc-support.c      |  2 +-
 cpukit/include/rtems/rtems/clockimpl.h | 14 +++++------
 cpukit/rtems/src/clockset.c            | 33 +++++++++++++-------------
 cpukit/rtems/src/clocktodvalidate.c    | 21 +++++++++-------
 cpukit/rtems/src/taskwakewhen.c        | 13 ++++++----
 cpukit/rtems/src/timercreate.c         | 10 +++++---
 testsuites/sptests/sp2038/init.c       |  6 ++---
 8 files changed, 59 insertions(+), 50 deletions(-)

diff --git a/bsps/arm/altera-cyclone-v/rtc/rtc.c b/bsps/arm/altera-cyclone-v/rtc/rtc.c
index 3e8c68e789..fb30da8d66 100644
--- a/bsps/arm/altera-cyclone-v/rtc/rtc.c
+++ b/bsps/arm/altera-cyclone-v/rtc/rtc.c
@@ -353,10 +353,9 @@ static int altera_cyclone_v_ds1339_get_time(int minor, rtems_time_of_day* tod)
     temp_tod.month  = ds1339_get_month(&time);
     temp_tod.year   = ds1339_get_year(&time);
 
-    if (_TOD_Validate(&temp_tod))
+    sc = _TOD_Validate(&temp_tod)
+    if (sc == RTEMS_SUCCESSFUL)
       memcpy(tod, &temp_tod, sizeof(temp_tod));
-    else
-      sc = RTEMS_INVALID_CLOCK;
   }
 
   return -sc;
@@ -737,10 +736,9 @@ static int  altera_cyclone_v_m41st87_get_time(int minor, rtems_time_of_day* tod)
   temp_tod.month  = m41st87_get_month(&time);
   temp_tod.year   = m41st87_get_year(&time);
 
-  if (_TOD_Validate(&temp_tod))
+  sc = _TOD_Validate(&temp_tod);
+  if (sc == RTEMS_SUCCESSFUL)
     memcpy(tod, &temp_tod, sizeof(temp_tod));
-  else
-    sc = RTEMS_INVALID_CLOCK;
 
   return -sc;
 }
diff --git a/bsps/shared/dev/rtc/rtc-support.c b/bsps/shared/dev/rtc/rtc-support.c
index 765bfe1d6b..04b8f0c847 100644
--- a/bsps/shared/dev/rtc/rtc-support.c
+++ b/bsps/shared/dev/rtc/rtc-support.c
@@ -255,7 +255,7 @@ int setRealTime(
   if (!RTC_Is_present())
     return -1;
 
-  if ( !_TOD_Validate(tod) )
+  if (_TOD_Validate(tod) != RTEMS_SUCCESSFUL)
     return -1;
 
   RTC_Table[RTC_Minor].pDeviceFns->deviceSetTime(RTC_Minor, tod);
diff --git a/cpukit/include/rtems/rtems/clockimpl.h b/cpukit/include/rtems/rtems/clockimpl.h
index c13c158410..8ec4f0f6e3 100644
--- a/cpukit/include/rtems/rtems/clockimpl.h
+++ b/cpukit/include/rtems/rtems/clockimpl.h
@@ -37,16 +37,16 @@ extern "C" {
 /**
  * @brief TOD Validate
  *
- * This support function returns true if @a the_tod contains
- * a valid time of day, and false otherwise.
+ * This support function tests whether @a the_tod references
+ * a valid time of day.
  *
- * @param[in] the_tod is the TOD structure to validate
+ * @param the_tod A reference to the time of day structure to validate.
  *
- * @retval This method returns true if the TOD is valid and false otherwise.
- *
- * @note This routine only works for leap-years through 2099.
+ * @retval RTEMS_SUCCESSFUL @a the_tod references a valid time of day.
+ * @retval RTEMS_INVALID_CLOCK @a the_tod references an invalid time of day.
+ * @retval RTEMS_INVALID_ADDRESS @a the_tod reference is @c NULL.
  */
-bool _TOD_Validate(
+rtems_status_code _TOD_Validate(
   const rtems_time_of_day *the_tod
 );
 
diff --git a/cpukit/rtems/src/clockset.c b/cpukit/rtems/src/clockset.c
index 7a085ada69..df163531a7 100644
--- a/cpukit/rtems/src/clockset.c
+++ b/cpukit/rtems/src/clockset.c
@@ -29,26 +29,25 @@ rtems_status_code rtems_clock_set(
   const rtems_time_of_day *tod
 )
 {
-  Status_Control status;
+  rtems_status_code status;
+  Status_Control    score_status;
+  struct timespec   tod_as_timespec;
+  ISR_lock_Context  lock_context;
 
-  if ( !tod )
-    return RTEMS_INVALID_ADDRESS;
+  status = _TOD_Validate( tod );
 
-  if ( _TOD_Validate( tod ) ) {
-    struct timespec  tod_as_timespec;
-    ISR_lock_Context lock_context;
-
-    tod_as_timespec.tv_sec = _TOD_To_seconds( tod );
-    tod_as_timespec.tv_nsec = tod->ticks
-      * rtems_configuration_get_nanoseconds_per_tick();
+  if ( status != RTEMS_SUCCESSFUL ) {
+    return status;
+  }
 
-    _TOD_Lock();
-    _TOD_Acquire( &lock_context );
-    status = _TOD_Set( &tod_as_timespec, &lock_context );
-    _TOD_Unlock();
+  tod_as_timespec.tv_sec = _TOD_To_seconds( tod );
+  tod_as_timespec.tv_nsec = tod->ticks
+    * rtems_configuration_get_nanoseconds_per_tick();
 
-    return _Status_Get( status );
-  }
+  _TOD_Lock();
+  _TOD_Acquire( &lock_context );
+  score_status = _TOD_Set( &tod_as_timespec, &lock_context );
+  _TOD_Unlock();
 
-  return RTEMS_INVALID_CLOCK;
+  return _Status_Get( score_status );
 }
diff --git a/cpukit/rtems/src/clocktodvalidate.c b/cpukit/rtems/src/clocktodvalidate.c
index 2685bfd6e7..d8e9d01382 100644
--- a/cpukit/rtems/src/clocktodvalidate.c
+++ b/cpukit/rtems/src/clocktodvalidate.c
@@ -35,17 +35,20 @@ const uint32_t   _TOD_Days_per_month[ 2 ][ 13 ] = {
   { 0, 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }
 };
 
-bool _TOD_Validate(
+rtems_status_code _TOD_Validate(
   const rtems_time_of_day *the_tod
 )
 {
   uint32_t   days_in_month;
   uint32_t   ticks_per_second;
 
+  if ( the_tod == NULL ) {
+    return RTEMS_INVALID_ADDRESS;
+  }
+
   ticks_per_second = TOD_MICROSECONDS_PER_SECOND /
 	    rtems_configuration_get_microseconds_per_tick();
-  if ((!the_tod)                                  ||
-      (the_tod->ticks  >= ticks_per_second)       ||
+  if ((the_tod->ticks  >= ticks_per_second)       ||
       (the_tod->second >= TOD_SECONDS_PER_MINUTE) ||
       (the_tod->minute >= TOD_MINUTES_PER_HOUR)   ||
       (the_tod->hour   >= TOD_HOURS_PER_DAY)      ||
@@ -53,8 +56,9 @@ bool _TOD_Validate(
       (the_tod->month  >  TOD_MONTHS_PER_YEAR)    ||
       (the_tod->year   <  TOD_BASE_YEAR)          ||
       (the_tod->year   >  TOD_LATEST_YEAR)        ||
-      (the_tod->day    == 0) )
-     return false;
+      (the_tod->day    == 0) ) {
+    return RTEMS_INVALID_CLOCK;
+  }
 
   if (((the_tod->year % 4) == 0 && (the_tod->year % 100 != 0)) ||
      (the_tod->year % 400 == 0))
@@ -62,8 +66,9 @@ bool _TOD_Validate(
   else
     days_in_month = _TOD_Days_per_month[ 0 ][ the_tod->month ];
 
-  if ( the_tod->day > days_in_month )
-    return false;
+  if ( the_tod->day > days_in_month ) {
+    return RTEMS_INVALID_CLOCK;
+  }
 
-  return true;
+  return RTEMS_SUCCESSFUL;
 }
diff --git a/cpukit/rtems/src/taskwakewhen.c b/cpukit/rtems/src/taskwakewhen.c
index 5f6a5795fc..a25204ad01 100644
--- a/cpukit/rtems/src/taskwakewhen.c
+++ b/cpukit/rtems/src/taskwakewhen.c
@@ -30,9 +30,10 @@ rtems_status_code rtems_task_wake_when(
   rtems_time_of_day *time_buffer
 )
 {
-  uint32_t         seconds;
-  Thread_Control  *executing;
-  Per_CPU_Control *cpu_self;
+  uint32_t          seconds;
+  Thread_Control   *executing;
+  Per_CPU_Control  *cpu_self;
+  rtems_status_code status;
 
   if ( !_TOD_Is_set() )
     return RTEMS_NOT_DEFINED;
@@ -41,9 +42,11 @@ rtems_status_code rtems_task_wake_when(
     return RTEMS_INVALID_ADDRESS;
 
   time_buffer->ticks = 0;
+  status = _TOD_Validate( time_buffer );
 
-  if ( !_TOD_Validate( time_buffer ) )
-    return RTEMS_INVALID_CLOCK;
+  if ( status != RTEMS_SUCCESSFUL ) {
+    return status;
+  }
 
   seconds = _TOD_To_seconds( time_buffer );
 
diff --git a/cpukit/rtems/src/timercreate.c b/cpukit/rtems/src/timercreate.c
index a3ece5cc4d..bd9421c440 100644
--- a/cpukit/rtems/src/timercreate.c
+++ b/cpukit/rtems/src/timercreate.c
@@ -132,7 +132,8 @@ rtems_status_code _Timer_Fire_when(
   Watchdog_Service_routine_entry     adaptor
 )
 {
-  rtems_interval seconds;
+  rtems_status_code status;
+  rtems_interval    seconds;
 
   if ( !_TOD_Is_set() )
     return RTEMS_NOT_DEFINED;
@@ -140,8 +141,11 @@ rtems_status_code _Timer_Fire_when(
   if ( !routine )
     return RTEMS_INVALID_ADDRESS;
 
-  if ( !_TOD_Validate( wall_time ) )
-    return RTEMS_INVALID_CLOCK;
+  status = _TOD_Validate( wall_time );
+
+  if ( status != RTEMS_SUCCESSFUL ) {
+    return status;
+  }
 
   seconds = _TOD_To_seconds( wall_time );
   if ( seconds <= _TOD_Seconds_since_epoch() )
diff --git a/testsuites/sptests/sp2038/init.c b/testsuites/sptests/sp2038/init.c
index 10850d9c4d..035b9a9b9b 100644
--- a/testsuites/sptests/sp2038/init.c
+++ b/testsuites/sptests/sp2038/init.c
@@ -277,14 +277,14 @@ static void test_problem_year(void)
 
 static void test_leap_year(void)
 {
-    bool test_status;
+    rtems_status_code test_status;
     const rtems_time_of_day *problem = &problem_2100;
     const rtems_time_of_day *problem2 = &problem_2100_2;
     // 2100 is not a leap year, so it should have 28 days
     test_status = _TOD_Validate(problem);
-    rtems_test_assert(test_status == true);
+    rtems_test_assert(test_status == RTEMS_SUCCESSFUL);
     test_status = _TOD_Validate(problem2);
-    rtems_test_assert(test_status == false);
+    rtems_test_assert(test_status == RTEMS_INVALID_CLOCK);
 }
 
 static bool test_year_is_leap_year(uint32_t year)
-- 
2.26.2



More information about the devel mailing list