The issue is not whether or not the application has an error.  Applications
can have all sorts of errors and it's not ODP's job to core dump on the
application's behalf whenever it suspects the application may have done
something wrong.  The issue is whether the API can reasonably return a
"sorry, I can't do that" return/error code.  If it can, then it should and
leave it up to the application to decide what to do in response to that.
 In the case of create/allocate type calls it is always possible for these
calls to fail (insufficient resources, etc.) so since the API is already
set up to report failures there's no reason not to use that mechanism for
general parameter issues as well.

Asserts in the ODP code itself are for debugging ODP implementations, not
for flagging parameter issues.  The specific exceptions we've made are in
performance paths where we don't want to spend pathlength validating
parameters that should be correct (e.g., API expects an odp_buffer_t and
expects caller to supply one that it hasn't mangled).  In this case the API
itself would likely segfault if garbage were passed in.


On Wed, Aug 27, 2014 at 7:15 AM, Savolainen, Petri (NSN - FI/Espoo) <
[email protected]> wrote:

>  E.g. exit() in alignment check after link_bufs indicates an internal
> error (bug in pool initialization code), and should not return an error
> code to the user.  User may miss to check for that and continue working
> with a broken pool.
>
>
>
> -Petri
>
>
>
> *From:* ext Mike Holmes [mailto:[email protected]]
> *Sent:* Wednesday, August 27, 2014 2:58 PM
> *To:* Savolainen, Petri (NSN - FI/Espoo)
> *Cc:* ext Bill Fischofer; Wiles, Roger Keith; lng-odp
>
> *Subject:* Re: [lng-odp] RFC odp_buffer_pool_create has no return code it
> calls exit(0)
>
>
>
> Summary of Platform Call discussion
>
>
>
> The exit call could be wrapped into ODP_ERR possibly so that at compile
> time other abortive action can be taken
>
> global_init could take a call back for absolutely fatal errors that
> ODP_ERR calls allowing the application to perform some action, it must not
> allow the app to continue running however
>
>
>
> We dont want to ever call exit directly in an implementation - add this to
> the new coding doc (comment added to do Mike)
>
>
>
> In this case there was a consensus that this is not a fatal error of the
> type that should call exit, and that it should return normally setting
> errno.
>
>
>
>
>
> On 27 August 2014 06:55, Mike Holmes <[email protected]> wrote:
>
> The key thing is to be consistent to the documentations description of the
> API I think, currently the odp_buffer_test behaves differently per
> platform, and neither are correct to the documentation.
>
>
>
> On 27 August 2014 05:31, Savolainen, Petri (NSN - FI/Espoo) <
> [email protected]> wrote:
>
> Hi,
>
>
>
> Some exit()s are caused by internal errors (which cannot be recovered
> from), so it might be better idea to crash right there rather than return
> an error code.
>
>
>
> -Petri
>
>
>
>
>
> *From:* ext Bill Fischofer [mailto:[email protected]]
> *Sent:* Wednesday, August 27, 2014 4:08 AM
> *To:* Wiles, Roger Keith
> *Cc:* Mike Holmes; Savolainen, Petri (NSN - FI/Espoo); lng-odp
> *Subject:* Re: [lng-odp] RFC odp_buffer_pool_create has no return code it
> calls exit(0)
>
>
>
> Many of the linux-generic routines that were written early in the project
> took various shortcuts in this area.  They need to be standardized as part
> of the ODP v1.0 cleanup/packaging/validation/documentation effort post-LCU.
>  We should start compiling a list of these and get BPs created for these
> now and at LCU.
>
>
>
> ODP routines, should not call exit() or similar global "give up" routines
> but should provide appropriate error returns in the case of most problems.
>  In the specific case of odp_buffer_pool_create(), it should never abort.
>  The parameter list also does not reflect the current design so that too
> will need to be harmonized.
>
>
>
> Bill
>
>
>
> On Tue, Aug 26, 2014 at 7:30 PM, Wiles, Roger Keith <
> [email protected]> wrote:
>
>
>
> On Aug 26, 2014, at 4:25 PM, Mike Holmes <[email protected]> wrote:
>
>
>
> The  odp_buffer_pool_create  function has a return code that defaults
> to  ODP_BUFFER_POOL_INVALID, although the API header file doxygen comment
> does not indicate that there is any error return code possible.
>
> I was going to add that information however there truly is no error
> returned because link_bufs actually calls exit(0) on error.
>
>
>
> My question is do we update odp_buffer_pool_create docs to say this
> function may never return, or should link_bufs be rewritten ?
>
>
>
> I would normally say we need to remove the exit(0)  but presumably this
> was done as as an optimization ?  In which case we just need to document it
> properly ?
>
>
>
> Thoughts ?
>
>
>
>
>
> odp_buffer_pool_t odp_buffer_pool_create(const char *name,
>
>                                          void *base_addr, uint64_t size,
>
>                                          size_t buf_size, size_t buf_align,
>
>                                          int buf_type)
>
> {
>
>         odp_buffer_pool_t i;
>
>         pool_entry_t *pool;
>
>         odp_buffer_pool_t pool_id = ODP_BUFFER_POOL_INVALID;
>
>
>
>         for (i = 0; i < ODP_CONFIG_BUFFER_POOLS; i++) {
>
>                 pool = get_pool_entry(i);
>
>
>
>                 LOCK(&pool->s.lock);
>
>
>
>                 if (pool->s.buf_base == 0) {
>
>                         /* found free pool */
>
>
>
>                         strncpy(pool->s.name, name,
>
>                                 ODP_BUFFER_POOL_NAME_LEN - 1);
>
>                         pool->s.name[ODP_BUFFER_POOL_NAME_LEN - 1] = 0;
>
>                         pool->s.pool_base_addr = base_addr;
>
>                         pool->s.pool_size      = size;
>
>                         pool->s.user_size      = buf_size;
>
>                         pool->s.user_align     = buf_align;
>
>                         pool->s.buf_type       = buf_type;
>
>
>
>                         link_bufs(pool); NEVER returns on some errors
>
>
>
> I would want this function to not do an exit() call without allowing the
> application to caught the error. It would mean the link_bufs() routine
> would need to return and error/error code instead of exiting. Having
> something exit is not a very friendly action IMO.
>
>
>
>                         UNLOCK(&pool->s.lock);
>
>
>
>                         pool_id = i;
>
>                         break;
>
>                 }
>
>
>
>                 UNLOCK(&pool->s.lock);
>
>         }
>
>
>
>         return pool_id;
>
> }
>
>
>
>
>
>
> --
>
> *Mike Holmes*
>
> Linaro Technical Manager / Lead
>
> LNG - ODP
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
> *Keith Wiles*, Principal Technologist with CTO office, *Wind River* mobile
> 972-213-5533
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
>
>
>
> --
>
> *Mike Holmes*
>
> Linaro Technical Manager / Lead
>
> LNG - ODP
>
>
>
>
>
> --
>
> *Mike Holmes*
>
> Linaro Technical Manager / Lead
>
> LNG - ODP
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to