diff mbox

[PATCHv2] linux-generic: timer fix odp_timer_pool_create return code

Message ID 1464366310-5066-1-git-send-email-maxim.uvarov@linaro.org
State Accepted
Commit eadb8722447f26b63c6d0c4d7a1968dd764bbdaa
Headers show

Commit Message

Maxim Uvarov May 27, 2016, 4:25 p.m. UTC 
Accodring to API return code for fail case is ODP_TIMER_POOL_INVALID
and errno set event if it's defined to NULL. Also add check on pool
alloc that input parameter is not invalid.
https://bugs.linaro.org/show_bug.cgi?id=2139

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v2: 1. ODP_ABORT - > ODP_ERR
     2. simplify return code (Bill, Ola)


 platform/linux-generic/odp_timer.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

Comments

Bill Fischofer May 27, 2016, 6:57 p.m. UTC | #1 
On Fri, May 27, 2016 at 11:25 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Accodring to API return code for fail case is ODP_TIMER_POOL_INVALID
> and errno set event if it's defined to NULL. Also add check on pool
> alloc that input parameter is not invalid.
> https://bugs.linaro.org/show_bug.cgi?id=2139
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>

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


> ---
>  v2: 1. ODP_ABORT - > ODP_ERR
>      2. simplify return code (Bill, Ola)
>
>
>  platform/linux-generic/odp_timer.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> index 4e56fb0..996edf0 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -221,16 +221,15 @@ static inline odp_timer_t tp_idx_to_handle(struct
> odp_timer_pool_s *tp,
>  static void itimer_init(odp_timer_pool *tp);
>  static void itimer_fini(odp_timer_pool *tp);
>
> -static odp_timer_pool *odp_timer_pool_new(
> -       const char *_name,
> -       const odp_timer_pool_param_t *param)
> +static odp_timer_pool_t odp_timer_pool_new(const char *_name,
> +                                          const odp_timer_pool_param_t
> *param)
>  {
>         uint32_t tp_idx = odp_atomic_fetch_add_u32(&num_timer_pools, 1);
>         if (odp_unlikely(tp_idx >= MAX_TIMER_POOLS)) {
>                 /* Restore the previous value */
>                 odp_atomic_sub_u32(&num_timer_pools, 1);
>                 __odp_errno = ENFILE; /* Table overflow */
> -               return NULL;
> +               return ODP_TIMER_POOL_INVALID;
>         }
>         size_t sz0 = ODP_ALIGN_ROUNDUP(sizeof(odp_timer_pool),
>                         ODP_CACHE_LINE_SIZE);
> @@ -794,10 +793,9 @@ odp_timer_pool_create(const char *name,
>         /* Verify that we have a valid (non-zero) timer resolution */
>         if (param->res_ns == 0) {
>                 __odp_errno = EINVAL;
> -               return NULL;
> +               return ODP_TIMER_POOL_INVALID;
>         }
> -       odp_timer_pool_t tp = odp_timer_pool_new(name, param);
> -       return tp;
> +       return odp_timer_pool_new(name, param);
>  }
>
>  void odp_timer_pool_start(void)
> @@ -845,17 +843,18 @@ odp_timer_t odp_timer_alloc(odp_timer_pool_t tpid,
>                             odp_queue_t queue,
>                             void *user_ptr)
>  {
> -       if (odp_unlikely(queue == ODP_QUEUE_INVALID))
> -               ODP_ABORT("%s: Invalid queue handle\n", tpid->name);
> +       if (odp_unlikely(tpid == ODP_TIMER_POOL_INVALID)) {
> +               ODP_ERR("Invalid timer pool.\n");
> +               return ODP_TIMER_INVALID;
> +       }
> +
> +       if (odp_unlikely(queue == ODP_QUEUE_INVALID)) {
> +               ODP_ERR("%s: Invalid queue handle\n", tpid->name);
> +               return ODP_TIMER_INVALID;
> +       }
>         /* We don't care about the validity of user_ptr because we will not
>          * attempt to dereference it */
> -       odp_timer_t hdl = timer_alloc(tpid, queue, user_ptr);
> -       if (odp_likely(hdl != ODP_TIMER_INVALID)) {
> -               /* Success */
> -               return hdl;
> -       }
> -       /* errno set by timer_alloc() */
> -       return ODP_TIMER_INVALID;
> +       return timer_alloc(tpid, queue, user_ptr);
>  }
>
>  odp_event_t odp_timer_free(odp_timer_t hdl)
> --
> 2.7.1.250.gff4ea60
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov May 30, 2016, 3:34 p.m. UTC | #2 
Merged,
Maxim.

On 05/27/16 21:57, Bill Fischofer wrote:
>
>
> On Fri, May 27, 2016 at 11:25 AM, Maxim Uvarov 
> <maxim.uvarov@linaro.org <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Accodring to API return code for fail case is ODP_TIMER_POOL_INVALID
>     and errno set event if it's defined to NULL. Also add check on pool
>     alloc that input parameter is not invalid.
>     https://bugs.linaro.org/show_bug.cgi?id=2139
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>
>
> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org 
> <mailto:bill.fischofer@linaro.org>>
>
>     ---
>      v2: 1. ODP_ABORT - > ODP_ERR
>          2. simplify return code (Bill, Ola)
>
>
>      platform/linux-generic/odp_timer.c | 31
>     +++++++++++++++----------------
>      1 file changed, 15 insertions(+), 16 deletions(-)
>
>     diff --git a/platform/linux-generic/odp_timer.c
>     b/platform/linux-generic/odp_timer.c
>     index 4e56fb0..996edf0 100644
>     --- a/platform/linux-generic/odp_timer.c
>     +++ b/platform/linux-generic/odp_timer.c
>     @@ -221,16 +221,15 @@ static inline odp_timer_t
>     tp_idx_to_handle(struct odp_timer_pool_s *tp,
>      static void itimer_init(odp_timer_pool *tp);
>      static void itimer_fini(odp_timer_pool *tp);
>
>     -static odp_timer_pool *odp_timer_pool_new(
>     -       const char *_name,
>     -       const odp_timer_pool_param_t *param)
>     +static odp_timer_pool_t odp_timer_pool_new(const char *_name,
>     +                                          const
>     odp_timer_pool_param_t *param)
>      {
>             uint32_t tp_idx =
>     odp_atomic_fetch_add_u32(&num_timer_pools, 1);
>             if (odp_unlikely(tp_idx >= MAX_TIMER_POOLS)) {
>                     /* Restore the previous value */
>                     odp_atomic_sub_u32(&num_timer_pools, 1);
>                     __odp_errno = ENFILE; /* Table overflow */
>     -               return NULL;
>     +               return ODP_TIMER_POOL_INVALID;
>             }
>             size_t sz0 = ODP_ALIGN_ROUNDUP(sizeof(odp_timer_pool),
>                             ODP_CACHE_LINE_SIZE);
>     @@ -794,10 +793,9 @@ odp_timer_pool_create(const char *name,
>             /* Verify that we have a valid (non-zero) timer resolution */
>             if (param->res_ns == 0) {
>                     __odp_errno = EINVAL;
>     -               return NULL;
>     +               return ODP_TIMER_POOL_INVALID;
>             }
>     -       odp_timer_pool_t tp = odp_timer_pool_new(name, param);
>     -       return tp;
>     +       return odp_timer_pool_new(name, param);
>      }
>
>      void odp_timer_pool_start(void)
>     @@ -845,17 +843,18 @@ odp_timer_t odp_timer_alloc(odp_timer_pool_t
>     tpid,
>                                 odp_queue_t queue,
>                                 void *user_ptr)
>      {
>     -       if (odp_unlikely(queue == ODP_QUEUE_INVALID))
>     -               ODP_ABORT("%s: Invalid queue handle\n", tpid->name);
>     +       if (odp_unlikely(tpid == ODP_TIMER_POOL_INVALID)) {
>     +               ODP_ERR("Invalid timer pool.\n");
>     +               return ODP_TIMER_INVALID;
>     +       }
>     +
>     +       if (odp_unlikely(queue == ODP_QUEUE_INVALID)) {
>     +               ODP_ERR("%s: Invalid queue handle\n", tpid->name);
>     +               return ODP_TIMER_INVALID;
>     +       }
>             /* We don't care about the validity of user_ptr because we
>     will not
>              * attempt to dereference it */
>     -       odp_timer_t hdl = timer_alloc(tpid, queue, user_ptr);
>     -       if (odp_likely(hdl != ODP_TIMER_INVALID)) {
>     -               /* Success */
>     -               return hdl;
>     -       }
>     -       /* errno set by timer_alloc() */
>     -       return ODP_TIMER_INVALID;
>     +       return timer_alloc(tpid, queue, user_ptr);
>      }
>
>      odp_event_t odp_timer_free(odp_timer_t hdl)
>     --
>     2.7.1.250.gff4ea60
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index 4e56fb0..996edf0 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -221,16 +221,15 @@  static inline odp_timer_t tp_idx_to_handle(struct odp_timer_pool_s *tp,
 static void itimer_init(odp_timer_pool *tp);
 static void itimer_fini(odp_timer_pool *tp);
 
-static odp_timer_pool *odp_timer_pool_new(
-	const char *_name,
-	const odp_timer_pool_param_t *param)
+static odp_timer_pool_t odp_timer_pool_new(const char *_name,
+					   const odp_timer_pool_param_t *param)
 {
 	uint32_t tp_idx = odp_atomic_fetch_add_u32(&num_timer_pools, 1);
 	if (odp_unlikely(tp_idx >= MAX_TIMER_POOLS)) {
 		/* Restore the previous value */
 		odp_atomic_sub_u32(&num_timer_pools, 1);
 		__odp_errno = ENFILE; /* Table overflow */
-		return NULL;
+		return ODP_TIMER_POOL_INVALID;
 	}
 	size_t sz0 = ODP_ALIGN_ROUNDUP(sizeof(odp_timer_pool),
 			ODP_CACHE_LINE_SIZE);
@@ -794,10 +793,9 @@  odp_timer_pool_create(const char *name,
 	/* Verify that we have a valid (non-zero) timer resolution */
 	if (param->res_ns == 0) {
 		__odp_errno = EINVAL;
-		return NULL;
+		return ODP_TIMER_POOL_INVALID;
 	}
-	odp_timer_pool_t tp = odp_timer_pool_new(name, param);
-	return tp;
+	return odp_timer_pool_new(name, param);
 }
 
 void odp_timer_pool_start(void)
@@ -845,17 +843,18 @@  odp_timer_t odp_timer_alloc(odp_timer_pool_t tpid,
 			    odp_queue_t queue,
 			    void *user_ptr)
 {
-	if (odp_unlikely(queue == ODP_QUEUE_INVALID))
-		ODP_ABORT("%s: Invalid queue handle\n", tpid->name);
+	if (odp_unlikely(tpid == ODP_TIMER_POOL_INVALID)) {
+		ODP_ERR("Invalid timer pool.\n");
+		return ODP_TIMER_INVALID;
+	}
+
+	if (odp_unlikely(queue == ODP_QUEUE_INVALID)) {
+		ODP_ERR("%s: Invalid queue handle\n", tpid->name);
+		return ODP_TIMER_INVALID;
+	}
 	/* We don't care about the validity of user_ptr because we will not
 	 * attempt to dereference it */
-	odp_timer_t hdl = timer_alloc(tpid, queue, user_ptr);
-	if (odp_likely(hdl != ODP_TIMER_INVALID)) {
-		/* Success */
-		return hdl;
-	}
-	/* errno set by timer_alloc() */
-	return ODP_TIMER_INVALID;
+	return timer_alloc(tpid, queue, user_ptr);
 }
 
 odp_event_t odp_timer_free(odp_timer_t hdl)