<div dir="ltr">Here you could add an assert that ptimer->clock_type ==<br><div>
CLOCK_MONOTONIC || ptimer->clock_type == CLOCK_REALTIME. <br></div><div><br></div><div>Would I need to assert that the timer is not a valid timer type? Also In psxtimer create I check if the timer is invalid. <br></div><div><br></div><div>I can do a similar if statement as I did in psxtimercreate</div><div><br></div><div><div style="color:rgb(212,212,212);background-color:rgb(30,30,30);font-family:"Droid Sans Mono","monospace",monospace,"Droid Sans Fallback";font-weight:normal;font-size:14px;line-height:19px;white-space:pre"><div><span style="color:rgb(212,212,212)"> </span><span style="color:rgb(197,134,192)">if</span><span style="color:rgb(212,212,212)"> ( </span><span style="color:rgb(156,220,254)">ptimer</span><span style="color:rgb(212,212,212)">-></span><span style="color:rgb(156,220,254)">clock_type</span><span style="color:rgb(212,212,212)"> != </span><span style="color:rgb(86,156,214)">CLOCK_REALTIME</span><span style="color:rgb(212,212,212)"> && </span><span style="color:rgb(156,220,254)">ptimer</span><span style="color:rgb(212,212,212)">-></span><span style="color:rgb(156,220,254)">clock_type</span><span style="color:rgb(212,212,212)"> != </span><span style="color:rgb(86,156,214)">CLOCK_MONOTONIC</span><span style="color:rgb(212,212,212)"> )</span></div><div><span style="color:rgb(212,212,212)"> </span><span style="color:rgb(220,220,170)">rtems_set_errno_and_return_minus_one</span><span style="color:rgb(212,212,212)">( </span><span style="color:rgb(86,156,214)">EINVAL</span><span style="color:rgb(212,212,212)"> );</span></div><div><span style="color:rgb(212,212,212)">Zack<br></span></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 4 Aug 2021 at 16:02, Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Jul 28, 2021 at 6:28 PM Zacchaeus Leung<br>
<<a href="mailto:zakthertemsdev@gmail.com" target="_blank">zakthertemsdev@gmail.com</a>> wrote:<br>
><br>
> the timer_create() method can use CLOCK_MONOTONIC but there was no test for this<br>
><br>
> Closes #3888<br>
> ---<br>
> cpukit/include/rtems/posix/timer.h    | 1 +<br>
> cpukit/posix/src/psxtimercreate.c     | 3 +-<br>
> cpukit/posix/src/timergettime.c      | 52 ++++++++++++++---------<br>
>Â testsuites/psxtests/psxtimer02/psxtimer.c | 26 ++++++++++++<br>
>Â 4 files changed, 60 insertions(+), 22 deletions(-)<br>
><br>
> diff --git a/cpukit/include/rtems/posix/timer.h b/cpukit/include/rtems/posix/timer.h<br>
> index bcbf07a65a..7ae089173a 100644<br>
> --- a/cpukit/include/rtems/posix/timer.h<br>
> +++ b/cpukit/include/rtems/posix/timer.h<br>
> @@ -48,6 +48,7 @@ typedef struct {<br>
>  uint32_t     ticks;   /* Number of ticks of the initialization */<br>
>  uint32_t     overrun;  /* Number of expirations of the timer  */<br>
>  struct timespec  time;    /* Time at which the timer was started  */<br>
> + clockid_t     clock_type; /* The type of timer */<br>
>Â } POSIX_Timer_Control;<br>
><br>
>Â /**<br>
> diff --git a/cpukit/posix/src/psxtimercreate.c b/cpukit/posix/src/psxtimercreate.c<br>
> index a63cf1d100..2b5a10140f 100644<br>
> --- a/cpukit/posix/src/psxtimercreate.c<br>
> +++ b/cpukit/posix/src/psxtimercreate.c<br>
> @@ -40,7 +40,7 @@ int timer_create(<br>
>Â {<br>
>Â Â POSIX_Timer_Control *ptimer;<br>
><br>
> -Â if ( clock_id != CLOCK_REALTIME )<br>
> +Â if (Â clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC )<br>
>Â Â Â rtems_set_errno_and_return_minus_one( EINVAL );<br>
><br>
>Â Â if ( !timerid )<br>
> @@ -91,6 +91,7 @@ int timer_create(<br>
>  ptimer->timer_data.it_value.tv_nsec  = 0;<br>
>  ptimer->timer_data.it_interval.tv_sec = 0;<br>
>Â Â ptimer->timer_data.it_interval.tv_nsec = 0;<br>
> +Â ptimer->clock_type = clock_id;<br>
><br>
>Â Â _Watchdog_Preinitialize( &ptimer->Timer, _Per_CPU_Get_snapshot() );<br>
>Â Â _Watchdog_Initialize( &ptimer->Timer, _POSIX_Timer_TSR );<br>
> diff --git a/cpukit/posix/src/timergettime.c b/cpukit/posix/src/timergettime.c<br>
> index ee2a566f0e..b29d03efa1 100644<br>
> --- a/cpukit/posix/src/timergettime.c<br>
> +++ b/cpukit/posix/src/timergettime.c<br>
> @@ -28,6 +28,7 @@<br>
>Â #include <rtems/score/todimpl.h><br>
>Â #include <rtems/score/watchdogimpl.h><br>
>Â #include <rtems/seterr.h><br>
> +#include <rtems/timespec.h><br>
><br>
>Â /*<br>
>Â Â *Â Â Â Â Â - When a timer is initialized, the value of the time in<br>
> @@ -39,35 +40,44 @@<br>
>Â int timer_gettime(<br>
>  timer_t      timerid,<br>
>Â Â struct itimerspec *value<br>
>Â )<br>
><br>
>Â {<br>
>Â Â POSIX_Timer_Control *ptimer;<br>
> - ISR_lock_Context   lock_context;<br>
> - uint64_t       now;<br>
> - uint32_t       remaining;<br>
> +Â ISR_lock_Context lock_context;<br>
> +Â Per_CPU_Control *cpu;<br>
> +Â struct timespec now;<br>
> +Â struct timespec expire;<br>
> +Â struct timespec result;<br>
><br>
>Â Â if ( !value )<br>
>Â Â Â rtems_set_errno_and_return_minus_one( EINVAL );<br>
><br>
>Â Â ptimer = _POSIX_Timer_Get( timerid, &lock_context );<br>
> -Â if ( ptimer != NULL ) {<br>
> -Â Â Per_CPU_Control *cpu;<br>
> -<br>
> -Â Â cpu = _POSIX_Timer_Acquire_critical( ptimer, &lock_context );<br>
> -Â Â now = cpu->Watchdog.ticks;<br>
> -<br>
> -Â Â if ( now < ptimer->Timer.expire ) {<br>
> -Â Â Â remaining = (uint32_t) ( ptimer->Timer.expire - now );<br>
> -Â Â } else {<br>
> -Â Â Â remaining = 0;<br>
> -Â Â }<br>
> +Â if ( ptimer == NULL ) {<br>
> +Â Â rtems_set_errno_and_return_minus_one( EINVAL );<br>
> +Â }<br>
><br>
> -Â Â _Timespec_From_ticks( remaining, &value->it_value );<br>
> -Â Â value->it_interval = ptimer->timer_data.it_interval;<br>
> +Â cpu = _POSIX_Timer_Acquire_critical( ptimer, &lock_context );<br>
> +Â rtems_timespec_from_ticks( ptimer->Timer.expire, &expire );<br>
><br>
> -Â Â _POSIX_Timer_Release( cpu, &lock_context );<br>
> -Â Â return 0;<br>
> +Â if ( ptimer->clock_type == CLOCK_MONOTONIC ) {<br>
> +Â _Timecounter_Nanouptime(&now);<br>
> +Â }<br>
> +Â if ( ptimer->clock_type == CLOCK_REALTIME ) {<br>
Minor nit: this could be 'else if'<br>
<br>
> +Â Â _TOD_Get(&now);<br>
>Â Â }<br>
Here you could add an assert that ptimer->clock_type ==<br>
CLOCK_MONOTONIC || ptimer->clock_type == CLOCK_REALTIME. Or you can<br>
add:<br>
 else {<br>
  _POSIX_Timer_Release( cpu, &lock_context );<br>
  rtems_set_errno_and_return_minus_one( EINVAL );<br>
 }<br>
and make sure to test by calling this function directly with an<br>
invalid clock_type. This of course depends on making the above into<br>
'else if'.<br>
<br>
><br>
> -Â rtems_set_errno_and_return_minus_one( EINVAL );<br>
> -}<br>
> +<br>
> + if ( rtems_timespec_less_than( &now, &expire ) ) {<br>
> +Â Â Â rtems_timespec_subtract( &now, &expire, &result );<br>
> +Â Â } else {<br>
> +Â Â Â result.tv_nsec = 0;<br>
> +   result.tv_sec = 0;<br>
> +Â Â }<br>
> +<br>
> +Â value->it_value=result;<br>
add spaces around =<br>
<br>
> +Â value->it_interval = ptimer->timer_data.it_interval;<br>
> +<br>
> +Â _POSIX_Timer_Release( cpu, &lock_context );<br>
> +Â return 0;<br>
> +}<br>
> \ No newline at end of file<br>
> diff --git a/testsuites/psxtests/psxtimer02/psxtimer.c b/testsuites/psxtests/psxtimer02/psxtimer.c<br>
> index 9f79d33c42..1a79369efb 100644<br>
> --- a/testsuites/psxtests/psxtimer02/psxtimer.c<br>
> +++ b/testsuites/psxtests/psxtimer02/psxtimer.c<br>
> @@ -126,6 +126,32 @@ void *POSIX_Init (<br>
>Â Â puts( "timer_delete - bad id - EINVAL" );<br>
>Â Â status = timer_delete( timer );<br>
>Â Â fatal_posix_service_status_errno( status, EINVAL, "bad id" );<br>
> +<br>
> +Â puts( "timer_create (monotonic) - bad timer id pointer - EINVAL" );<br>
> +Â status = timer_create( CLOCK_MONOTONIC, &event, NULL );<br>
> +Â fatal_posix_service_status_errno( status, EINVAL, "bad timer id" );<br>
> +<br>
> +Â puts( "timer_create (monotonic) - OK" );<br>
> +Â status = timer_create( CLOCK_MONOTONIC, NULL, &timer );<br>
> +Â posix_service_failed( status, "timer_create OK" );<br>
> +<br>
> +Â puts( "timer_create (monotonic) - too many - EAGAIN" );<br>
> +Â status = timer_create( CLOCK_MONOTONIC, NULL, &timer1 );<br>
> +Â fatal_posix_service_status_errno( status, EAGAIN, "too many" );<br>
> +<br>
> +Â clock_gettime( CLOCK_MONOTONIC, &now );<br>
> +Â itimer.it_value = now;<br>
> +Â itimer.it_value.tv_sec = itimer.it_value.tv_sec - 1;<br>
> +Â puts( "timer_settime (monotonic) - bad itimer value - previous time - EINVAL" );<br>
> +Â status = timer_settime( timer, TIMER_ABSTIME, &itimer, NULL );<br>
> +Â fatal_posix_service_status_errno( status, EINVAL, "bad itimer value #3" );<br>
> +<br>
> +Â clock_gettime( CLOCK_MONOTONIC, &now );<br>
> +Â itimer.it_value = now;<br>
> +Â itimer.it_value.tv_sec = itimer.it_value.tv_sec + 1;<br>
> +Â puts( "timer_settime (monotonic) - bad id - EINVAL" );<br>
> +Â status = timer_settime( timer1, TIMER_ABSTIME, &itimer, NULL );<br>
> +Â fatal_posix_service_status_errno( status, EINVAL, "bad id" );<br>
><br>
>Â Â TEST_END();<br>
>Â Â rtems_test_exit (0);<br>
> --<br>
> 2.32.0<br>
><br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div>