diff mbox

[API-NEXT,PATCHv3] linux-generic: time: remove posix bleed through on odp_time_t

Message ID 566FF39C.2050302@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Dec. 15, 2015, 11:03 a.m. UTC
On 12/15/2015 01:15, Bill Fischofer wrote:
> v3 posted to fix doxygen issues.

1. In generated doc it looks like comment should go under @note. It will 
be good to put notes to
the latest string in the section like other api do, but I don't know how 
to do it.

2. Time structure Used to.."

3. Might be it's reasonable to hide sec and nsec in doc.




>
> On Mon, Dec 14, 2015 at 4:14 PM, Bill Fischofer 
> <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote:
>
>     The linux-generic implementation of odp_time_t makes use of POSIX
>     APIs that are sensitive to the _POSIX_C_SOURCE level. Use an
>     indirection
>     mechanism so that these dependencies do not "bleed through" the
>     ODP API.
>     This means that ODP applications can be independent of _POSIX_C_SOURCE
>     level.
>
>     Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org
>     <mailto:bill.fischofer@linaro.org>>
>     ---
>      .../linux-generic/include/odp/plat/time_types.h    | 14 +++++++---
>      platform/linux-generic/odp_time.c                  | 30
>     +++++++++++-----------
>      2 files changed, 25 insertions(+), 19 deletions(-)
>
>     diff --git a/platform/linux-generic/include/odp/plat/time_types.h
>     b/platform/linux-generic/include/odp/plat/time_types.h
>     index e5765ec..6269e40 100644
>     --- a/platform/linux-generic/include/odp/plat/time_types.h
>     +++ b/platform/linux-generic/include/odp/plat/time_types.h
>     @@ -21,11 +21,17 @@ extern "C" {
>       *  @{
>       **/
>
>     -typedef struct timespec odp_time_t;
>     -
>     -odp_time_t odp_time_null(void);
>     +/**
>     + * Time structure
>     + * Used to isolate linux-generic implementation from the linux
>     + * timespec structure, which is dependent on _POSIX_C_SOURCE level.
>     + */
>     +typedef struct odp_time_t {
>     +       int64_t tv_sec;      /**< Seconds */
>     +       int64_t tv_nsec;     /**< Nanoseconds */
>     +} odp_time_t;
>
>     -#define ODP_TIME_NULL  odp_time_null()
>     +#define ODP_TIME_NULL ((odp_time_t){0, 0})
>
>      /**
>       * @}
>     diff --git a/platform/linux-generic/odp_time.c
>     b/platform/linux-generic/odp_time.c
>     index 1c7c214..9b64b37 100644
>     --- a/platform/linux-generic/odp_time.c
>     +++ b/platform/linux-generic/odp_time.c
>     @@ -11,7 +11,12 @@
>      #include <odp/hints.h>
>      #include <odp_debug_internal.h>
>
>     -static struct timespec start_time;
>     +typedef union {
>     +       odp_time_t      ex;
>     +       struct timespec in;
>     +} _odp_time_t;
>     +
>     +static odp_time_t start_time;
>
>      static inline
>      uint64_t time_to_ns(odp_time_t time)
>     @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
>      static inline
>      odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>      {
>     -       struct timespec time;
>     +       odp_time_t time;
>
>             time.tv_sec = t2.tv_sec - t1.tv_sec;
>             time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
>     @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>      odp_time_t odp_time_local(void)
>      {
>             int ret;
>     -       struct timespec time;
>     +       _odp_time_t time;
>
>     -       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
>     +       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in
>     <http://time.in>);
>             if (odp_unlikely(ret != 0))
>                     ODP_ABORT("clock_gettime failed\n");
>
>     -       return time_diff(time, start_time);
>     +       return time_diff(time.ex, start_time);
>      }
>
>      odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
>     @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
>
>      odp_time_t odp_time_local_from_ns(uint64_t ns)
>      {
>     -       struct timespec time;
>     +       odp_time_t time;
>
>             time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
>             time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
>     @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
>
>      odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
>      {
>     -       struct timespec time;
>     +       odp_time_t time;
>
>             time.tv_sec = t2.tv_sec + t1.tv_sec;
>             time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
>     @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
>             return time_to_ns(time) / resolution;
>      }
>
>     -odp_time_t odp_time_null(void)
>     -{
>     -       return (struct timespec) {0, 0};
>     -}
>     -
>      int odp_time_global_init(void)
>      {
>             int ret;
>     -       struct timespec time;
>     +       _odp_time_t time;
>
>     -       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
>     -       start_time = ret ? odp_time_null() : time;
>     +       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in
>     <http://time.in>);
>     +       start_time = ret ? ODP_TIME_NULL : time.ex;
>
>             return ret;
>      }
>     --
>     2.1.4
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp

Comments

Bill Fischofer Dec. 15, 2015, 11:56 a.m. UTC | #1
v4 sent to add @intenral as requested.

On Tue, Dec 15, 2015 at 5:16 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia.com> wrote:

>

>

> > -----Original Message-----

> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT

> > Maxim Uvarov

> > Sent: Tuesday, December 15, 2015 1:04 PM

> > To: lng-odp@lists.linaro.org

> > Subject: Re: [lng-odp] [API-NEXT PATCHv3] linux-generic: time: remove

> > posix bleed through on odp_time_t

> >

> > On 12/15/2015 01:15, Bill Fischofer wrote:

> > > v3 posted to fix doxygen issues.

> >

> > 1. In generated doc it looks like comment should go under @note. It will

> > be good to put notes to

> > the latest string in the section like other api do, but I don't know how

> > to do it.

> >

> > 2. Time structure Used to.."

> >

> > 3. Might be it's reasonable to hide sec and nsec in doc.

> >

> >

> > --- a/platform/linux-generic/include/odp/plat/time_types.h

> > +++ b/platform/linux-generic/include/odp/plat/time_types.h

> > @@ -22,13 +22,13 @@ extern "C" {

> >    **/

> >

> >   /**

> > - * Time structure

> > - * Used to isolate linux-generic implementation from the linux

> > - * timespec structure, which is dependent on _POSIX_C_SOURCE level.

> > + * @note Time structure used to isolate linux-generic implementation

> > + * from the linux timespec structure, which is dependent on

> > + * _POSIX_C_SOURCE level.

>

> The struct is internal to linux-generic. Better to hide it also (use

> @internal, instead of @note).

>

> -Petri

>

> >    */

> >   typedef struct odp_time_t {

> > -       int64_t tv_sec;      /**< Seconds */

> > -       int64_t tv_nsec;     /**< Nanoseconds */

> > +       int64_t tv_sec;      /**< @internal Seconds */

> > +       int64_t tv_nsec;     /**< @internal Nanoseconds */

> >   } odp_time_t;

> >

> >   #define ODP_TIME_NULL ((odp_time_t){0, 0})

> >

> >

> > >

> > > On Mon, Dec 14, 2015 at 4:14 PM, Bill Fischofer

> > > <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote:

> > >

> > >     The linux-generic implementation of odp_time_t makes use of POSIX

> > >     APIs that are sensitive to the _POSIX_C_SOURCE level. Use an

> > >     indirection

> > >     mechanism so that these dependencies do not "bleed through" the

> > >     ODP API.

> > >     This means that ODP applications can be independent of

> > _POSIX_C_SOURCE

> > >     level.

> > >

> > >     Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org

> > >     <mailto:bill.fischofer@linaro.org>>

> > >     ---

> > >      .../linux-generic/include/odp/plat/time_types.h    | 14 +++++++---

> > >      platform/linux-generic/odp_time.c                  | 30

> > >     +++++++++++-----------

> > >      2 files changed, 25 insertions(+), 19 deletions(-)

> > >

> > >     diff --git a/platform/linux-generic/include/odp/plat/time_types.h

> > >     b/platform/linux-generic/include/odp/plat/time_types.h

> > >     index e5765ec..6269e40 100644

> > >     --- a/platform/linux-generic/include/odp/plat/time_types.h

> > >     +++ b/platform/linux-generic/include/odp/plat/time_types.h

> > >     @@ -21,11 +21,17 @@ extern "C" {

> > >       *  @{

> > >       **/

> > >

> > >     -typedef struct timespec odp_time_t;

> > >     -

> > >     -odp_time_t odp_time_null(void);

> > >     +/**

> > >     + * Time structure

> > >     + * Used to isolate linux-generic implementation from the linux

> > >     + * timespec structure, which is dependent on _POSIX_C_SOURCE

> level.

> > >     + */

> > >     +typedef struct odp_time_t {

> > >     +       int64_t tv_sec;      /**< Seconds */

> > >     +       int64_t tv_nsec;     /**< Nanoseconds */

> > >     +} odp_time_t;

> > >

> > >     -#define ODP_TIME_NULL  odp_time_null()

> > >     +#define ODP_TIME_NULL ((odp_time_t){0, 0})

> > >

> > >      /**

> > >       * @}

> > >     diff --git a/platform/linux-generic/odp_time.c

> > >     b/platform/linux-generic/odp_time.c

> > >     index 1c7c214..9b64b37 100644

> > >     --- a/platform/linux-generic/odp_time.c

> > >     +++ b/platform/linux-generic/odp_time.c

> > >     @@ -11,7 +11,12 @@

> > >      #include <odp/hints.h>

> > >      #include <odp_debug_internal.h>

> > >

> > >     -static struct timespec start_time;

> > >     +typedef union {

> > >     +       odp_time_t      ex;

> > >     +       struct timespec in;

> > >     +} _odp_time_t;

> > >     +

> > >     +static odp_time_t start_time;

> > >

> > >      static inline

> > >      uint64_t time_to_ns(odp_time_t time)

> > >     @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)

> > >      static inline

> > >      odp_time_t time_diff(odp_time_t t2, odp_time_t t1)

> > >      {

> > >     -       struct timespec time;

> > >     +       odp_time_t time;

> > >

> > >             time.tv_sec = t2.tv_sec - t1.tv_sec;

> > >             time.tv_nsec = t2.tv_nsec - t1.tv_nsec;

> > >     @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t

> > t1)

> > >      odp_time_t odp_time_local(void)

> > >      {

> > >             int ret;

> > >     -       struct timespec time;

> > >     +       _odp_time_t time;

> > >

> > >     -       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);

> > >     +       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in

> > >     <http://time.in>);

> > >             if (odp_unlikely(ret != 0))

> > >                     ODP_ABORT("clock_gettime failed\n");

> > >

> > >     -       return time_diff(time, start_time);

> > >     +       return time_diff(time.ex, start_time);

> > >      }

> > >

> > >      odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)

> > >     @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)

> > >

> > >      odp_time_t odp_time_local_from_ns(uint64_t ns)

> > >      {

> > >     -       struct timespec time;

> > >     +       odp_time_t time;

> > >

> > >             time.tv_sec = ns / ODP_TIME_SEC_IN_NS;

> > >             time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;

> > >     @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)

> > >

> > >      odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)

> > >      {

> > >     -       struct timespec time;

> > >     +       odp_time_t time;

> > >

> > >             time.tv_sec = t2.tv_sec + t1.tv_sec;

> > >             time.tv_nsec = t2.tv_nsec + t1.tv_nsec;

> > >     @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)

> > >             return time_to_ns(time) / resolution;

> > >      }

> > >

> > >     -odp_time_t odp_time_null(void)

> > >     -{

> > >     -       return (struct timespec) {0, 0};

> > >     -}

> > >     -

> > >      int odp_time_global_init(void)

> > >      {

> > >             int ret;

> > >     -       struct timespec time;

> > >     +       _odp_time_t time;

> > >

> > >     -       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);

> > >     -       start_time = ret ? odp_time_null() : time;

> > >     +       ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in

> > >     <http://time.in>);

> > >     +       start_time = ret ? ODP_TIME_NULL : time.ex;

> > >

> > >             return ret;

> > >      }

> > >     --

> > >     2.1.4

> > >

> > >

> > >

> > >

> > > _______________________________________________

> > > lng-odp mailing list

> > > lng-odp@lists.linaro.org

> > > https://lists.linaro.org/mailman/listinfo/lng-odp

> >

> > _______________________________________________

> > lng-odp mailing list

> > lng-odp@lists.linaro.org

> > https://lists.linaro.org/mailman/listinfo/lng-odp

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
diff mbox

Patch

--- a/platform/linux-generic/include/odp/plat/time_types.h
+++ b/platform/linux-generic/include/odp/plat/time_types.h
@@ -22,13 +22,13 @@  extern "C" {
   **/

  /**
- * Time structure
- * Used to isolate linux-generic implementation from the linux
- * timespec structure, which is dependent on _POSIX_C_SOURCE level.
+ * @note Time structure used to isolate linux-generic implementation
+ * from the linux timespec structure, which is dependent on
+ * _POSIX_C_SOURCE level.
   */
  typedef struct odp_time_t {
-       int64_t tv_sec;      /**< Seconds */
-       int64_t tv_nsec;     /**< Nanoseconds */
+       int64_t tv_sec;      /**< @internal Seconds */
+       int64_t tv_nsec;     /**< @internal Nanoseconds */
  } odp_time_t;

  #define ODP_TIME_NULL ((odp_time_t){0, 0})