[PATCH 1/3] _TOD_Validate(): Fix incorrect return code
Gedare Bloom
gedare at rtems.org
Tue May 11 20:00:45 UTC 2021
On Tue, May 11, 2021 at 9:38 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> 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)
These three files in bsps/ should not be calling score functions
directly, unless they implement part of the score CPU port. These
don't seem to be doing that. This issue exists before this patch, but
a ticket should be opened to fix this. A new API should be added for
application/BSP layers to validate an rtems_time_of_day value.
> 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
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list