[PATCH v2] c-user: Document new clock manager directives

[Chris Johns]

On 9/11/21 5:57 pm, Sebastian Huber wrote:
> On 09/11/2021 03:46, Chris Johns wrote:
>>
>> On 20/10/21 2:25 am, Sebastian Huber wrote:
>>> Add new clock manager directives to get all times provided by the
>>> timehands.
>>>
>>> Update #4527.
>>> ---
>>> For an updated document to review see:
>>>
>>> https://ftp.rtems.org/pub/rtems/people/sebh/c-user.pdf
>>>
>>> v2: Clarify boot time.
>>>
>>>   c-user/clock/directives.rst   | 835 ++++++++++++++++++++++++++++++++++
>>>   c-user/clock/introduction.rst |  81 ++++
>>>   2 files changed, 916 insertions(+)
>>>
>>> diff --git a/c-user/clock/directives.rst b/c-user/clock/directives.rst
>>> index bdb7680..8f2d7ea 100644
>>> --- a/c-user/clock/directives.rst
>>> +++ b/c-user/clock/directives.rst
>>> @@ -224,6 +224,841 @@ The following constraints apply to this directive:
>>>     Applications which are restricted to only use interfaces of the
>>> pre-qualified
>>>     feature set of RTEMS shall not use the directive.
>>>   +.. Generated from spec:/rtems/clock/if/get-realtime
>>> +
>>> +.. raw:: latex
>>> +
>>> +    \clearpage
>>> +
>>> +.. index:: rtems_clock_get_realtime()
>>> +
>>> +.. _InterfaceRtemsClockGetRealtime:
>>> +
>>> +rtems_clock_get_realtime()
>>> +--------------------------
>>> +
>>> +Gets the time elapsed since the :term:`Unix epoch` measured using
>>> +:term:`CLOCK_REALTIME` in seconds and nanoseconds format.
>>> +
>>> +.. rubric:: CALLING SEQUENCE:
>>> +
>>> +.. code-block:: c
>>> +
>>> +    void rtems_clock_get_realtime( struct timespec *time_snapshot );
>>> +
>>> +.. rubric:: PARAMETERS:
>>> +
>>> +``time_snapshot``
>>> +    This parameter is the pointer to a `struct timespec
>>> +<https://en.cppreference.com/w/c/chrono/timespec>`_ object. The time> +   
>>> elapsed since the :term:`Unix epoch` measured using the
>>> +    :term:`CLOCK_REALTIME` at some time point during the directive call will be
>>> +    stored in this object.  Calling the directive with a pointer equal to `NULL
>>> +<https://en.cppreference.com/w/c/types/NULL>`_ is undefined behaviour.
>> Why not return an invalid error status? Same question for the same thing below.
> 
> I just documented the existing implementation. If you want an error status and
> NULL pointer checks, then we have to add a wrapper function around the FreeBSD
> interface. We still have to export the FreeBSD interface since it is used by
> libbsd.

Sure. I was only comparing this against the other APIs we have and a NULL is
checked.

> 
> The functions just have a single parameter and no return value currently. Why
> would someone pass a NULL pointer to such functions?
> 
> We could also change
> 
> RTEMS_ALIAS(_Timecounter_Nanotime)
> void rtems_clock_get_realtime(struct timespec *);
> 
> to
> 
> rtems_status_code
> rtems_clock_get_realtime(struct timespec *time_snapshot)
> {
>   if ( time_snapshot == NULL ) {
>     return RTEMS_INVALID_ADDRESS;
>   }
> 
>   _Timecounter_Nanotime( time_snapshot );
>   return RTEMS_SUCCESSFUL;
> }
> 
> However, this is less efficient. You need an extra comparison, a branch, a stack
> frame, push/pop of the return address, and an extra function call.
> 
> We could also use something like this:
> 
> static inline struct timespec rtems_clock_get_realtime(void)
> {
>   struct timespec time_snapshot;
> 
>   _Timecounter_Nanotime( &time_snapshot );
> 
>   return time_snapshot;
> }
> 
> Unfortunately GCC is not able to optimize this.
> 

Ah OK. This can be fixed and the performance improved but once the API is set it
cannot change or do you think we can add a check later and not break the API?

Chris