On 06/06/16 23:16, Bill Fischofer wrote:
I disagree with this. The enum in this case is simply an alternative to using #defines. These values are in fact ints and ODP API standard practice is to return ints, not enums. I don't see any advantage to singling out these APIs for different treatment.

One could argue that the "proper" thing to do here would be to return -1 and have the specific reason be returned via the odp_errno. But I don't think that change is needed either since we define <0 RCs to be error returns and its up to each API to decide if it wants to have multiple such error return values.
Bill, in doxygen description we did not define any <0 RCs, only that we have in enum.

So if it's int probably we need to add:
 * @retval other != 0 return for other error */


/**
 * Set a timer (absolute time) with a user-provided timeout event
 *
 * Set (arm) the timer to expire at specific time. The timeout
 * event will be enqueued when the timer expires.
 *
 * @param tim      Timer
 * @param abs_tck  Expiration time in absolute timer ticks
 * @param[in,out] tmo_ev  Reference to an event variable that points to
 * timeout event or NULL to reuse the existing timeout event. Any existing
 * timeout event that is replaced by a successful set operation will be
 * returned here.
 *
 * @retval ODP_TIMER_SUCCESS Operation succeeded
 * @retval ODP_TIMER_TOOEARLY Operation failed because expiration tick too
 * early
 * @retval ODP_TIMER_TOOLATE Operation failed because expiration tick too
 * late
 * @retval ODP_TIMER_NOEVENT Operation failed because timeout event not
 * specified in odp_timer_set call and not present in timer
 */
int odp_timer_set_abs(odp_timer_t tim,
              uint64_t abs_tck,
              odp_event_t *tmo_ev);


Maxim.

On Mon, Jun 6, 2016 at 2:28 PM, Maxim Uvarov <[email protected] <mailto:[email protected]>> wrote:

    For some reason we defined function to set timer as int but have enum
    in api. It looks more clear to bind return code to current api
    description.

    Signed-off-by: Maxim Uvarov <[email protected]
    <mailto:[email protected]>>
    ---
     include/odp/api/spec/timer.h       | 12 ++++++------
     platform/linux-generic/odp_timer.c | 12 ++++++------
     2 files changed, 12 insertions(+), 12 deletions(-)

    diff --git a/include/odp/api/spec/timer.h
    b/include/odp/api/spec/timer.h
    index 3f8fdd4..2c3bb8a 100644
    --- a/include/odp/api/spec/timer.h
    +++ b/include/odp/api/spec/timer.h
    @@ -243,9 +243,9 @@ odp_event_t odp_timer_free(odp_timer_t tim);
      * @retval ODP_TIMER_NOEVENT Operation failed because timeout
    event not
      * specified in odp_timer_set call and not present in timer
      */
    -int odp_timer_set_abs(odp_timer_t tim,
    -                     uint64_t abs_tck,
    -                     odp_event_t *tmo_ev);
    +odp_timer_set_t odp_timer_set_abs(odp_timer_t tim,
    +                                 uint64_t abs_tck,
    +                                 odp_event_t *tmo_ev);

     /**
      * Set a timer with a relative expiration time and user-provided
    event.
    @@ -268,9 +268,9 @@ int odp_timer_set_abs(odp_timer_t tim,
      * @retval ODP_TIMER_NOEVENT Operation failed because timeout
    event not
      * specified in call and not present in timer
      */
    -int odp_timer_set_rel(odp_timer_t tim,
    -                     uint64_t rel_tck,
    -                     odp_event_t *tmo_ev);
    +odp_timer_set_t odp_timer_set_rel(odp_timer_t tim,
    +                                 uint64_t rel_tck,
    +                                 odp_event_t *tmo_ev);

     /**
      * Cancel a timer
    diff --git a/platform/linux-generic/odp_timer.c
    b/platform/linux-generic/odp_timer.c
    index 996edf0..595b6ab 100644
    --- a/platform/linux-generic/odp_timer.c
    +++ b/platform/linux-generic/odp_timer.c
    @@ -865,9 +865,9 @@ odp_event_t odp_timer_free(odp_timer_t hdl)
            return odp_buffer_to_event(old_buf);
     }

    -int odp_timer_set_abs(odp_timer_t hdl,
    -                     uint64_t abs_tck,
    -                     odp_event_t *tmo_ev)
    +odp_timer_set_t odp_timer_set_abs(odp_timer_t hdl,
    +                                 uint64_t abs_tck,
    +                                 odp_event_t *tmo_ev)
     {
            odp_timer_pool *tp = handle_to_tp(hdl);
            uint32_t idx = handle_to_idx(hdl, tp);
    @@ -882,9 +882,9 @@ int odp_timer_set_abs(odp_timer_t hdl,
                    return ODP_TIMER_NOEVENT;
     }

    -int odp_timer_set_rel(odp_timer_t hdl,
    -                     uint64_t rel_tck,
    -                     odp_event_t *tmo_ev)
    +odp_timer_set_t odp_timer_set_rel(odp_timer_t hdl,
    +                                 uint64_t rel_tck,
    +                                 odp_event_t *tmo_ev)
     {
            odp_timer_pool *tp = handle_to_tp(hdl);
            uint32_t idx = handle_to_idx(hdl, tp);
    --
    2.7.1.250.gff4ea60

    _______________________________________________
    lng-odp mailing list
    [email protected] <mailto:[email protected]>
    https://lists.linaro.org/mailman/listinfo/lng-odp



_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to