[PATCH v2] rtems: Constify rtems_task_wake_when()

Gedare Bloom gedare at rtems.org
Wed May 12 16:46:25 UTC 2021


I understand the efficiency, but the interface is awkward. At least
maybe put UINT32_MAX behind a macro to make the intent more clear
#define RTEMS_TICKS_MASK 0xffffffffU
or UINT32_MAX is fine there too, equivalent.

Otherwise, go ahead.

On Tue, May 11, 2021 at 10:46 PM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> Add a parameter to _TOD_Validate() to disable the validation of the
> ticks member.
>
> There are two reasons for this change.  Firstly, in
> rtems_task_wake_when() was a double check for time_buffer == NULL (one
> in rtems_task_wake_when() and one in _TOD_Validate()).  Secondly, the
> ticks member is ignored by rtems_task_wake_when().  This was done with a
> write of zero to the ticks member and thus a modification of the
> user-provided structure.  Now the structure is no longer modified.
> Using a mask parameter is quite efficient. You just have to load an
> immediate value and there are no additional branches in _TOD_Validate().
>
> Close #4406.
> ---
>  bsps/arm/altera-cyclone-v/rtc/rtc.c    |  3 +--
>  bsps/shared/dev/rtc/rtc-support.c      |  2 +-
>  cpukit/include/rtems/rtems/clockimpl.h | 13 ++++++++-----
>  cpukit/include/rtems/rtems/tasks.h     |  2 +-
>  cpukit/rtems/src/clockset.c            |  2 +-
>  cpukit/rtems/src/clocktodvalidate.c    |  5 +++--
>  cpukit/rtems/src/taskwakewhen.c        |  8 ++------
>  cpukit/rtems/src/timercreate.c         |  2 +-
>  testsuites/sptests/sp2038/init.c       |  4 ++--
>  9 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/bsps/arm/altera-cyclone-v/rtc/rtc.c b/bsps/arm/altera-cyclone-v/rtc/rtc.c
> index fb30da8d66..4edc7c87f6 100644
> --- a/bsps/arm/altera-cyclone-v/rtc/rtc.c
> +++ b/bsps/arm/altera-cyclone-v/rtc/rtc.c
> @@ -345,7 +345,6 @@ static int altera_cyclone_v_ds1339_get_time(int minor, rtems_time_of_day* tod)
>
>    if (sc == RTEMS_SUCCESSFUL)
>    {
> -    temp_tod.ticks  = 0;
>      temp_tod.second = ds1339_get_seconds(&time);
>      temp_tod.minute = ds1339_get_minutes(&time);
>      temp_tod.hour   = ds1339_get_hours(&time);
> @@ -353,7 +352,7 @@ 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);
>
> -    sc = _TOD_Validate(&temp_tod)
> +    sc = _TOD_Validate(&temp_tod, 0)
>      if (sc == RTEMS_SUCCESSFUL)
>        memcpy(tod, &temp_tod, sizeof(temp_tod));
>    }
> diff --git a/bsps/shared/dev/rtc/rtc-support.c b/bsps/shared/dev/rtc/rtc-support.c
> index 04b8f0c847..1a0f2a3d9a 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) != RTEMS_SUCCESSFUL)
> +  if (_TOD_Validate(tod, UINT32_MAX) != 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 8ec4f0f6e3..670ea1e3d7 100644
> --- a/cpukit/include/rtems/rtems/clockimpl.h
> +++ b/cpukit/include/rtems/rtems/clockimpl.h
> @@ -35,19 +35,22 @@ extern "C" {
>   */
>
>  /**
> - * @brief TOD Validate
> + * @brief Validates the time of day.
>   *
> - * This support function tests whether @a the_tod references
> - * a valid time of day.
> + * @param the_tod is the reference to the time of day structure to validate or
> + *   NULL.
>   *
> - * @param the_tod A reference to the time of day structure to validate.
> + * @param ticks_mask is the mask for the ticks member of the time of day.  Use
> + *   UINT32_MAX to validate the ticks member.  Use zero to skip the validation of
> + *   the ticks member.
>   *
>   * @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.
>   */
>  rtems_status_code _TOD_Validate(
> -  const rtems_time_of_day *the_tod
> +  const rtems_time_of_day *the_tod,
> +  uint32_t                 ticks_mask
>  );
>
>  /**
> diff --git a/cpukit/include/rtems/rtems/tasks.h b/cpukit/include/rtems/rtems/tasks.h
> index e6f7696923..841306eaef 100644
> --- a/cpukit/include/rtems/rtems/tasks.h
> +++ b/cpukit/include/rtems/rtems/tasks.h
> @@ -2071,7 +2071,7 @@ rtems_status_code rtems_task_wake_after( rtems_interval ticks );
>   *   occur.
>   * @endparblock
>   */
> -rtems_status_code rtems_task_wake_when( rtems_time_of_day *time_buffer );
> +rtems_status_code rtems_task_wake_when( const rtems_time_of_day *time_buffer );
>
>  /* Generated from spec:/rtems/task/if/get-scheduler */
>
> diff --git a/cpukit/rtems/src/clockset.c b/cpukit/rtems/src/clockset.c
> index df163531a7..d79d7648e8 100644
> --- a/cpukit/rtems/src/clockset.c
> +++ b/cpukit/rtems/src/clockset.c
> @@ -34,7 +34,7 @@ rtems_status_code rtems_clock_set(
>    struct timespec   tod_as_timespec;
>    ISR_lock_Context  lock_context;
>
> -  status = _TOD_Validate( tod );
> +  status = _TOD_Validate( tod, UINT32_MAX );
>
>    if ( status != RTEMS_SUCCESSFUL ) {
>      return status;
> diff --git a/cpukit/rtems/src/clocktodvalidate.c b/cpukit/rtems/src/clocktodvalidate.c
> index 95c8e77c37..41a1167287 100644
> --- a/cpukit/rtems/src/clocktodvalidate.c
> +++ b/cpukit/rtems/src/clocktodvalidate.c
> @@ -36,7 +36,8 @@ const uint32_t   _TOD_Days_per_month[ 2 ][ 13 ] = {
>  };
>
>  rtems_status_code _TOD_Validate(
> -  const rtems_time_of_day *the_tod
> +  const rtems_time_of_day *the_tod,
> +  uint32_t                 ticks_mask
>  )
>  {
>    uint32_t   days_in_month;
> @@ -48,7 +49,7 @@ rtems_status_code _TOD_Validate(
>
>    ticks_per_second = rtems_clock_get_ticks_per_second();
>
> -  if ((the_tod->ticks  >= ticks_per_second)       ||
> +  if ( ( ( the_tod->ticks & ticks_mask ) >= ticks_per_second ) ||
>        (the_tod->second >= TOD_SECONDS_PER_MINUTE) ||
>        (the_tod->minute >= TOD_MINUTES_PER_HOUR)   ||
>        (the_tod->hour   >= TOD_HOURS_PER_DAY)      ||
> diff --git a/cpukit/rtems/src/taskwakewhen.c b/cpukit/rtems/src/taskwakewhen.c
> index a25204ad01..110debfec2 100644
> --- a/cpukit/rtems/src/taskwakewhen.c
> +++ b/cpukit/rtems/src/taskwakewhen.c
> @@ -27,7 +27,7 @@
>  #include <rtems/score/watchdogimpl.h>
>
>  rtems_status_code rtems_task_wake_when(
> -  rtems_time_of_day *time_buffer
> +  const rtems_time_of_day *time_buffer
>  )
>  {
>    uint32_t          seconds;
> @@ -38,11 +38,7 @@ rtems_status_code rtems_task_wake_when(
>    if ( !_TOD_Is_set() )
>      return RTEMS_NOT_DEFINED;
>
> -  if ( !time_buffer )
> -    return RTEMS_INVALID_ADDRESS;
> -
> -  time_buffer->ticks = 0;
> -  status = _TOD_Validate( time_buffer );
> +  status = _TOD_Validate( time_buffer, 0 );
>
>    if ( status != RTEMS_SUCCESSFUL ) {
>      return status;
> diff --git a/cpukit/rtems/src/timercreate.c b/cpukit/rtems/src/timercreate.c
> index bd9421c440..9624a3ff62 100644
> --- a/cpukit/rtems/src/timercreate.c
> +++ b/cpukit/rtems/src/timercreate.c
> @@ -141,7 +141,7 @@ rtems_status_code _Timer_Fire_when(
>    if ( !routine )
>      return RTEMS_INVALID_ADDRESS;
>
> -  status = _TOD_Validate( wall_time );
> +  status = _TOD_Validate( wall_time, UINT32_MAX );
>
>    if ( status != RTEMS_SUCCESSFUL ) {
>      return status;
> diff --git a/testsuites/sptests/sp2038/init.c b/testsuites/sptests/sp2038/init.c
> index 035b9a9b9b..31af29758b 100644
> --- a/testsuites/sptests/sp2038/init.c
> +++ b/testsuites/sptests/sp2038/init.c
> @@ -281,9 +281,9 @@ static void test_leap_year(void)
>      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);
> +    test_status = _TOD_Validate(problem, UINT32_MAX);
>      rtems_test_assert(test_status == RTEMS_SUCCESSFUL);
> -    test_status = _TOD_Validate(problem2);
> +    test_status = _TOD_Validate(problem2, UINT32_MAX);
>      rtems_test_assert(test_status == RTEMS_INVALID_CLOCK);
>  }
>
> --
> 2.26.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list