diff mbox

[v2,1/2] linux-generic: timer: use strong typing

Message ID 1462374164-29676-1-git-send-email-matias.elo@nokia.com
State Superseded
Headers show

Commit Message

Elo, Matias (Nokia - FI/Espoo) May 4, 2016, 3:02 p.m. UTC 
Use strong typing for odp_timer_t and odp_timeout_t. Fix
couple exposed usage errors.

Signed-off-by: Matias Elo <matias.elo@nokia.com>
---
 example/timer/odp_timer_test.c                            |  8 ++++----
 platform/linux-generic/include/odp/api/plat/timer_types.h | 10 ++++++----
 platform/linux-generic/odp_timer.c                        |  6 +++---
 test/validation/timer/timer.c                             |  2 +-
 4 files changed, 14 insertions(+), 12 deletions(-)

Comments

Bill Fischofer May 4, 2016, 4:06 p.m. UTC | #1 
For this series:

Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

On Wed, May 4, 2016 at 10:02 AM, Matias Elo <matias.elo@nokia.com> wrote:

> Use strong typing for odp_timer_t and odp_timeout_t. Fix

> couple exposed usage errors.

>

> Signed-off-by: Matias Elo <matias.elo@nokia.com>

> ---

>  example/timer/odp_timer_test.c                            |  8 ++++----

>  platform/linux-generic/include/odp/api/plat/timer_types.h | 10 ++++++----

>  platform/linux-generic/odp_timer.c                        |  6 +++---

>  test/validation/timer/timer.c                             |  2 +-

>  4 files changed, 14 insertions(+), 12 deletions(-)

>

> diff --git a/example/timer/odp_timer_test.c

> b/example/timer/odp_timer_test.c

> index f1d3be4..c07d6ed 100644

> --- a/example/timer/odp_timer_test.c

> +++ b/example/timer/odp_timer_test.c

> @@ -165,16 +165,16 @@ static void test_abs_timeouts(int thr,

> test_globals_t *gbls)

>                 if (!odp_timeout_fresh(tmo)) {

>                         /* Not the expected expiration tick, timer has

>                          * been reset or cancelled or freed */

> -                       EXAMPLE_ABORT("Unexpected timeout received (timer

> %" PRIx32 ", tick %" PRIu64 ")\n",

> -                                     ttp->tim, tick);

> +                       EXAMPLE_ABORT("Unexpected timeout received (timer

> %" PRIu64 ", tick %" PRIu64 ")\n",

> +                                     odp_timer_to_u64(ttp->tim), tick);

>                 }

>                 EXAMPLE_DBG("  [%i] timeout, tick %"PRIu64"\n", thr, tick);

>

>                 uint32_t rx_num = odp_atomic_fetch_dec_u32(&gbls->remain);

>

>                 if (!rx_num)

> -                       EXAMPLE_ABORT("Unexpected timeout received (timer

> %x, tick %"PRIu64")\n",

> -                                     ttp->tim, tick);

> +                       EXAMPLE_ABORT("Unexpected timeout received (timer

> %" PRIu64 ", tick %" PRIu64 ")\n",

> +                                     odp_timer_to_u64(ttp->tim), tick);

>                 else if (rx_num > num_workers)

>                         continue;

>

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

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

> index 006683e..93ea162 100644

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

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

> @@ -18,6 +18,8 @@

>  extern "C" {

>  #endif

>

> +#include <odp/api/plat/strong_types.h>

> +

>  /** @addtogroup odp_timer

>   *  @{

>   **/

> @@ -28,13 +30,13 @@ typedef struct odp_timer_pool_s *odp_timer_pool_t;

>

>  #define ODP_TIMER_POOL_INVALID NULL

>

> -typedef uint32_t odp_timer_t;

> +typedef ODP_HANDLE_T(odp_timer_t);

>

> -#define ODP_TIMER_INVALID ((uint32_t)~0U)

> +#define ODP_TIMER_INVALID _odp_cast_scalar(odp_timer_t, 0xffffffff)

>

> -typedef void *odp_timeout_t;

> +typedef ODP_HANDLE_T(odp_timeout_t);

>

> -#define ODP_TIMEOUT_INVALID NULL

> +#define ODP_TIMEOUT_INVALID  _odp_cast_scalar(odp_timeout_t, 0)

>

>  /**

>   * @}

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

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

> index f4fb1f6..9e21f3a 100644

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

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

> @@ -184,7 +184,7 @@ static odp_timer_pool *timer_pool[MAX_TIMER_POOLS];

>

>  static inline odp_timer_pool *handle_to_tp(odp_timer_t hdl)

>  {

> -       uint32_t tp_idx = hdl >> INDEX_BITS;

> +       uint32_t tp_idx = _odp_typeval(hdl) >> INDEX_BITS;

>         if (odp_likely(tp_idx < MAX_TIMER_POOLS)) {

>                 odp_timer_pool *tp = timer_pool[tp_idx];

>                 if (odp_likely(tp != NULL))

> @@ -196,7 +196,7 @@ static inline odp_timer_pool *handle_to_tp(odp_timer_t

> hdl)

>  static inline uint32_t handle_to_idx(odp_timer_t hdl,

>                 struct odp_timer_pool_s *tp)

>  {

> -       uint32_t idx = hdl & ((1U << INDEX_BITS) - 1U);

> +       uint32_t idx = _odp_typeval(hdl) & ((1U << INDEX_BITS) - 1U);

>         PREFETCH(&tp->tick_buf[idx]);

>         if (odp_likely(idx < odp_atomic_load_u32(&tp->high_wm)))

>                 return idx;

> @@ -207,7 +207,7 @@ static inline odp_timer_t tp_idx_to_handle(struct

> odp_timer_pool_s *tp,

>                 uint32_t idx)

>  {

>         ODP_ASSERT(idx < (1U << INDEX_BITS));

> -       return (tp->tp_idx << INDEX_BITS) | idx;

> +       return _odp_cast_scalar(odp_timer_t, (tp->tp_idx << INDEX_BITS) |

> idx);

>  }

>

>  /* Forward declarations */

> diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c

> index aad11aa..1032adc 100644

> --- a/test/validation/timer/timer.c

> +++ b/test/validation/timer/timer.c

> @@ -297,7 +297,7 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)

>                 if (tt[i].tim == ODP_TIMER_INVALID) {

>                         LOG_DBG("Failed to allocate timer (%" PRIu32

> "/%d)\n",

>                                 i, NTIMERS);

> -                       odp_timeout_free(tt[i].ev);

> +                       odp_event_free(tt[i].ev);

>                         break;

>                 }

>                 tt[i].ev2 = tt[i].ev;

> --

> 1.9.1

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

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

>
Maxim Uvarov May 11, 2016, 3:36 p.m. UTC | #2 
this can be simple split on 2 lines:

WARNING: line over 80 characters
#29: FILE: example/timer/odp_timer_test.c:168:
+            EXAMPLE_ABORT("Unexpected timeout received (timer %" PRIu64 
", tick %" PRIu64 ")\n",

WARNING: line over 80 characters
#39: FILE: example/timer/odp_timer_test.c:176:
+            EXAMPLE_ABORT("Unexpected timeout received (timer %" PRIu64 
", tick %" PRIu64 ")\

On 05/04/16 19:06, Bill Fischofer wrote:
> For this series:
>
> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org 
> <mailto:bill.fischofer@linaro.org>>
>
> On Wed, May 4, 2016 at 10:02 AM, Matias Elo <matias.elo@nokia.com 
> <mailto:matias.elo@nokia.com>> wrote:
>
>     Use strong typing for odp_timer_t and odp_timeout_t. Fix
>     couple exposed usage errors.
>
>     Signed-off-by: Matias Elo <matias.elo@nokia.com
>     <mailto:matias.elo@nokia.com>>
>     ---
>      example/timer/odp_timer_test.c |  8 ++++----
>      platform/linux-generic/include/odp/api/plat/timer_types.h | 10
>     ++++++----
>      platform/linux-generic/odp_timer.c |  6 +++---
>      test/validation/timer/timer.c  |  2 +-
>      4 files changed, 14 insertions(+), 12 deletions(-)
>
>     diff --git a/example/timer/odp_timer_test.c
>     b/example/timer/odp_timer_test.c
>     index f1d3be4..c07d6ed 100644
>     --- a/example/timer/odp_timer_test.c
>     +++ b/example/timer/odp_timer_test.c
>     @@ -165,16 +165,16 @@ static void test_abs_timeouts(int thr,
>     test_globals_t *gbls)
>                     if (!odp_timeout_fresh(tmo)) {
>                             /* Not the expected expiration tick, timer has
>                              * been reset or cancelled or freed */
>     -                       EXAMPLE_ABORT("Unexpected timeout received
>     (timer %" PRIx32 ", tick %" PRIu64 ")\n",
>     -                                     ttp->tim, tick);
>     +                       EXAMPLE_ABORT("Unexpected timeout received
>     (timer %" PRIu64 ", tick %" PRIu64 ")\n",
>     +  odp_timer_to_u64(ttp->tim), tick);
>                     }
>                     EXAMPLE_DBG("  [%i] timeout, tick %"PRIu64"\n",
>     thr, tick);
>
>                     uint32_t rx_num =
>     odp_atomic_fetch_dec_u32(&gbls->remain);
>
>                     if (!rx_num)
>     -                       EXAMPLE_ABORT("Unexpected timeout received
>     (timer %x, tick %"PRIu64")\n",
>     -                                     ttp->tim, tick);
>     +                       EXAMPLE_ABORT("Unexpected timeout received
>     (timer %" PRIu64 ", tick %" PRIu64 ")\n",
>     +  odp_timer_to_u64(ttp->tim), tick);
>                     else if (rx_num > num_workers)
>                             continue;
>
>     diff --git
>     a/platform/linux-generic/include/odp/api/plat/timer_types.h
>     b/platform/linux-generic/include/odp/api/plat/timer_types.h
>     index 006683e..93ea162 100644
>     --- a/platform/linux-generic/include/odp/api/plat/timer_types.h
>     +++ b/platform/linux-generic/include/odp/api/plat/timer_types.h
>     @@ -18,6 +18,8 @@
>      extern "C" {
>      #endif
>
>     +#include <odp/api/plat/strong_types.h>
>     +
>      /** @addtogroup odp_timer
>       *  @{
>       **/
>     @@ -28,13 +30,13 @@ typedef struct odp_timer_pool_s *odp_timer_pool_t;
>
>      #define ODP_TIMER_POOL_INVALID NULL
>
>     -typedef uint32_t odp_timer_t;
>     +typedef ODP_HANDLE_T(odp_timer_t);
>
>     -#define ODP_TIMER_INVALID ((uint32_t)~0U)
>     +#define ODP_TIMER_INVALID _odp_cast_scalar(odp_timer_t, 0xffffffff)
>
>     -typedef void *odp_timeout_t;
>     +typedef ODP_HANDLE_T(odp_timeout_t);
>
>     -#define ODP_TIMEOUT_INVALID NULL
>     +#define ODP_TIMEOUT_INVALID _odp_cast_scalar(odp_timeout_t, 0)
>
>      /**
>       * @}
>     diff --git a/platform/linux-generic/odp_timer.c
>     b/platform/linux-generic/odp_timer.c
>     index f4fb1f6..9e21f3a 100644
>     --- a/platform/linux-generic/odp_timer.c
>     +++ b/platform/linux-generic/odp_timer.c
>     @@ -184,7 +184,7 @@ static odp_timer_pool
>     *timer_pool[MAX_TIMER_POOLS];
>
>      static inline odp_timer_pool *handle_to_tp(odp_timer_t hdl)
>      {
>     -       uint32_t tp_idx = hdl >> INDEX_BITS;
>     +       uint32_t tp_idx = _odp_typeval(hdl) >> INDEX_BITS;
>             if (odp_likely(tp_idx < MAX_TIMER_POOLS)) {
>                     odp_timer_pool *tp = timer_pool[tp_idx];
>                     if (odp_likely(tp != NULL))
>     @@ -196,7 +196,7 @@ static inline odp_timer_pool
>     *handle_to_tp(odp_timer_t hdl)
>      static inline uint32_t handle_to_idx(odp_timer_t hdl,
>                     struct odp_timer_pool_s *tp)
>      {
>     -       uint32_t idx = hdl & ((1U << INDEX_BITS) - 1U);
>     +       uint32_t idx = _odp_typeval(hdl) & ((1U << INDEX_BITS) - 1U);
>             PREFETCH(&tp->tick_buf[idx]);
>             if (odp_likely(idx < odp_atomic_load_u32(&tp->high_wm)))
>                     return idx;
>     @@ -207,7 +207,7 @@ static inline odp_timer_t
>     tp_idx_to_handle(struct odp_timer_pool_s *tp,
>                     uint32_t idx)
>      {
>             ODP_ASSERT(idx < (1U << INDEX_BITS));
>     -       return (tp->tp_idx << INDEX_BITS) | idx;
>     +       return _odp_cast_scalar(odp_timer_t, (tp->tp_idx <<
>     INDEX_BITS) | idx);
>      }
>
>      /* Forward declarations */
>     diff --git a/test/validation/timer/timer.c
>     b/test/validation/timer/timer.c
>     index aad11aa..1032adc 100644
>     --- a/test/validation/timer/timer.c
>     +++ b/test/validation/timer/timer.c
>     @@ -297,7 +297,7 @@ static void *worker_entrypoint(void *arg
>     TEST_UNUSED)
>                     if (tt[i].tim == ODP_TIMER_INVALID) {
>                             LOG_DBG("Failed to allocate timer (%"
>     PRIu32 "/%d)\n",
>                                     i, NTIMERS);
>     -                       odp_timeout_free(tt[i].ev);
>     +                       odp_event_free(tt[i].ev);
>                             break;
>                     }
>                     tt[i].ev2 = tt[i].ev;
>     --
>     1.9.1
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto: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

diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c
index f1d3be4..c07d6ed 100644
--- a/example/timer/odp_timer_test.c
+++ b/example/timer/odp_timer_test.c
@@ -165,16 +165,16 @@  static void test_abs_timeouts(int thr, test_globals_t *gbls)
 		if (!odp_timeout_fresh(tmo)) {
 			/* Not the expected expiration tick, timer has
 			 * been reset or cancelled or freed */
-			EXAMPLE_ABORT("Unexpected timeout received (timer %" PRIx32 ", tick %" PRIu64 ")\n",
-				      ttp->tim, tick);
+			EXAMPLE_ABORT("Unexpected timeout received (timer %" PRIu64 ", tick %" PRIu64 ")\n",
+				      odp_timer_to_u64(ttp->tim), tick);
 		}
 		EXAMPLE_DBG("  [%i] timeout, tick %"PRIu64"\n", thr, tick);
 
 		uint32_t rx_num = odp_atomic_fetch_dec_u32(&gbls->remain);
 
 		if (!rx_num)
-			EXAMPLE_ABORT("Unexpected timeout received (timer %x, tick %"PRIu64")\n",
-				      ttp->tim, tick);
+			EXAMPLE_ABORT("Unexpected timeout received (timer %" PRIu64 ", tick %" PRIu64 ")\n",
+				      odp_timer_to_u64(ttp->tim), tick);
 		else if (rx_num > num_workers)
 			continue;
 
diff --git a/platform/linux-generic/include/odp/api/plat/timer_types.h b/platform/linux-generic/include/odp/api/plat/timer_types.h
index 006683e..93ea162 100644
--- a/platform/linux-generic/include/odp/api/plat/timer_types.h
+++ b/platform/linux-generic/include/odp/api/plat/timer_types.h
@@ -18,6 +18,8 @@ 
 extern "C" {
 #endif
 
+#include <odp/api/plat/strong_types.h>
+
 /** @addtogroup odp_timer
  *  @{
  **/
@@ -28,13 +30,13 @@  typedef struct odp_timer_pool_s *odp_timer_pool_t;
 
 #define ODP_TIMER_POOL_INVALID NULL
 
-typedef uint32_t odp_timer_t;
+typedef ODP_HANDLE_T(odp_timer_t);
 
-#define ODP_TIMER_INVALID ((uint32_t)~0U)
+#define ODP_TIMER_INVALID _odp_cast_scalar(odp_timer_t, 0xffffffff)
 
-typedef void *odp_timeout_t;
+typedef ODP_HANDLE_T(odp_timeout_t);
 
-#define ODP_TIMEOUT_INVALID NULL
+#define ODP_TIMEOUT_INVALID  _odp_cast_scalar(odp_timeout_t, 0)
 
 /**
  * @}
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index f4fb1f6..9e21f3a 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -184,7 +184,7 @@  static odp_timer_pool *timer_pool[MAX_TIMER_POOLS];
 
 static inline odp_timer_pool *handle_to_tp(odp_timer_t hdl)
 {
-	uint32_t tp_idx = hdl >> INDEX_BITS;
+	uint32_t tp_idx = _odp_typeval(hdl) >> INDEX_BITS;
 	if (odp_likely(tp_idx < MAX_TIMER_POOLS)) {
 		odp_timer_pool *tp = timer_pool[tp_idx];
 		if (odp_likely(tp != NULL))
@@ -196,7 +196,7 @@  static inline odp_timer_pool *handle_to_tp(odp_timer_t hdl)
 static inline uint32_t handle_to_idx(odp_timer_t hdl,
 		struct odp_timer_pool_s *tp)
 {
-	uint32_t idx = hdl & ((1U << INDEX_BITS) - 1U);
+	uint32_t idx = _odp_typeval(hdl) & ((1U << INDEX_BITS) - 1U);
 	PREFETCH(&tp->tick_buf[idx]);
 	if (odp_likely(idx < odp_atomic_load_u32(&tp->high_wm)))
 		return idx;
@@ -207,7 +207,7 @@  static inline odp_timer_t tp_idx_to_handle(struct odp_timer_pool_s *tp,
 		uint32_t idx)
 {
 	ODP_ASSERT(idx < (1U << INDEX_BITS));
-	return (tp->tp_idx << INDEX_BITS) | idx;
+	return _odp_cast_scalar(odp_timer_t, (tp->tp_idx << INDEX_BITS) | idx);
 }
 
 /* Forward declarations */
diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
index aad11aa..1032adc 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -297,7 +297,7 @@  static void *worker_entrypoint(void *arg TEST_UNUSED)
 		if (tt[i].tim == ODP_TIMER_INVALID) {
 			LOG_DBG("Failed to allocate timer (%" PRIu32 "/%d)\n",
 				i, NTIMERS);
-			odp_timeout_free(tt[i].ev);
+			odp_event_free(tt[i].ev);
 			break;
 		}
 		tt[i].ev2 = tt[i].ev;