Why are we not defining odp_fatal_error() in the public ODP API and call that function directly? What's the benefit of an additional layer of macro?
On 28 August 2014 14:56, Bill Fischofer <[email protected]> wrote: > Based on today's meeting, the recommendation is that ODP code calls > ODP_ERR() in the event of a fatal internal error. ODP_ERR() will then call > an application-supplied fatal error handler. If no such handler is > supplied, then it will call abort(). In any event, ODP_ERR() will not > return. > > I would write it as a macro: > > #define ODP_ERR(x) do {odp_fatal_error(x);} while(1) > > Bill > > > On Thu, Aug 28, 2014 at 6:46 AM, Mike Holmes <[email protected]> > wrote: > >> >> >> On 28 August 2014 03:13, Savolainen, Petri (NSN - FI/Espoo) < >> [email protected]> wrote: >> >>> Sure, just trying to make the point here that all exit() calls cannot >>> be find-and-replaced with return + error code. It’s better to crash on >>> internal errors (like the one below). >>> >>> >>> >> >> What is the conclusion. >> This code should call ODP_ERR which for now prints up a message and then >> calls abort ? >> >> Do we need to add a bug to update the code and the the docs ? >> >> >> >>> -Petri >>> >>> >>> >>> *From:* ext Ola Liljedahl [mailto:[email protected]] >>> *Sent:* Wednesday, August 27, 2014 6:12 PM >>> >>> *To:* Savolainen, Petri (NSN - FI/Espoo) >>> *Cc:* ext Mike Holmes; lng-odp >>> >>> *Subject:* Re: [lng-odp] RFC odp_buffer_pool_create has no return code >>> it calls exit(0) >>> >>> >>> >>> For internal errors, I really think abort() should be called by the ODP >>> implementation. Someone will have to debug and fix this problem, a silent >>> exit is not helpful. A core dump is a good start. >>> >>> >>> >>> On 27 August 2014 14:15, 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 >>> >>> >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
