| Message ID | 1397666579-9600-2-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Apr 16, 2014 at 11:42 AM, Mike Holmes <mike.holmes@linaro.org>wrote: > > +if (unrecoverable_out_of_foos == 1) > +{ > + ODP_ERR("Completely unable to proceed, no foos available"); > + tidy_op_for_exit(); > + ... > +} > > The typo I pointed out last time is still there. Shouldn't it be tidy_up_for_exit() rather than tidy_op_for_exit()? The latter doesn't scan. Bill
On Wed, 16 Apr 2014 12:42:58 -0400 Mike Holmes <mike.holmes@linaro.org> wrote: > +@subsection compile_time_examples Examples > +Checking size and alignment of a struct with offsetof > + > +@code > +typedef struct timer timer; > +struct timer > +{ > + uint8_t MODE; > + uint32_t DATA; > + uint32_t COUNT; > +}; > + > + > +#if (offsetof(timer, DATA) != 4) > +#error DATA must be at offset 4 in timer > +#endif > +@endcode IIRC, offsetof and friends aren't available in the c preprocessor - [/me goes and checks] yep, this code fails to build: offsetofincpp.c:10:14: error: missing binary operator before token "(" #if (offsetof(timer, DATA) != 4) ^ make: *** [offsetofincpp] Error 1 fwiw, you can stop a compiler dead in its tracks by declaring an array of negative length -> something a build-time ODP macro might want to do if its condition is true, esp. if you want to avoid the preprocessor and the limited ugliness it imposes on the code, as shown above. > +@section compile_run_time Run time programming exceptions > +There are two rules > +-# These must be capable of being turned off by defining -DODP_NO_DEBUG > +-# They must use ODP_ASSERT so that the output may be redirected on systems without stderr. > +-# ODP_ASSERT will call abort() as its final operation. > + > +@note ODP_ASSERT is defined to make it easier to redirect output from stderr. For example > +an in memory text buffer may be in use if stderr has no meaning on a bare metal implimentation > + > +@subsection compile_run_time_examples Examples > +Checks that the API function arguments are within the permitted value range (e.g. handle validation > + > +@code > +void odp_foo(char *pointer) > +{ > + ODP_ASSERT(pointer != NULL); > + … > +} > +@endcode > +*/ personally, I'd rather either "if (!ptr) return", "if (!ptr) { printf ("moron!\n"); return; }" (i.e., favour continued execution), or use gdb for these types of things: it's not like ODP is closed source... Last but not least: code residing in documentation should always conform to codingstyle: DATA->data, etc. Kim
Thanks Bill, missed that one. If there is nothing else I will push this tomorrow with the above typo fixed. On 16 April 2014 12:50, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > > On Wed, Apr 16, 2014 at 11:42 AM, Mike Holmes <mike.holmes@linaro.org>wrote: > >> >> +if (unrecoverable_out_of_foos == 1) >> +{ >> + ODP_ERR("Completely unable to proceed, no foos available"); >> + tidy_op_for_exit(); >> + ... >> +} >> >> The typo I pointed out last time is still there. Shouldn't it be > tidy_up_for_exit() rather than tidy_op_for_exit()? The latter doesn't scan. > > Bill >
Thanks Kim On 16 April 2014 14:42, Kim Phillips <kim.phillips@freescale.com> wrote: > On Wed, 16 Apr 2014 12:42:58 -0400 > Mike Holmes <mike.holmes@linaro.org> wrote: > > > +@subsection compile_time_examples Examples > > +Checking size and alignment of a struct with offsetof > > + > > +@code > > +typedef struct timer timer; > > +struct timer > > +{ > > + uint8_t MODE; > > + uint32_t DATA; > > + uint32_t COUNT; > > +}; > > + > > + > > +#if (offsetof(timer, DATA) != 4) > > +#error DATA must be at offset 4 in timer > > +#endif > > +@endcode > > IIRC, offsetof and friends aren't available in the c preprocessor - > [/me goes and checks] yep, this code fails to build: > > offsetofincpp.c:10:14: error: missing binary operator before token "(" > #if (offsetof(timer, DATA) != 4) > ^ > make: *** [offsetofincpp] Error 1 > > Ok, need to make a new example for the doc. > fwiw, you can stop a compiler dead in its tracks by declaring > an array of negative length -> something a build-time ODP macro > might want to do if its condition is true, esp. if you want to avoid > the preprocessor and the limited ugliness it imposes on the code, as > shown above. > > > +@section compile_run_time Run time programming exceptions > > +There are two rules > > +-# These must be capable of being turned off by defining -DODP_NO_DEBUG > > +-# They must use ODP_ASSERT so that the output may be redirected on > systems without stderr. > > +-# ODP_ASSERT will call abort() as its final operation. > > + > > +@note ODP_ASSERT is defined to make it easier to redirect output from > stderr. For example > > +an in memory text buffer may be in use if stderr has no meaning on a > bare metal implimentation > > + > > +@subsection compile_run_time_examples Examples > > +Checks that the API function arguments are within the permitted value > range (e.g. handle validation > > + > > +@code > > +void odp_foo(char *pointer) > > +{ > > + ODP_ASSERT(pointer != NULL); > > + … > > +} > > +@endcode > > +*/ > > personally, I'd rather either "if (!ptr) return", "if (!ptr) { printf > ("moron!\n"); return; }" (i.e., favour continued execution), or use > gdb for these types of things: it's not like ODP is closed source... > I think Asserts are supposed to be for things that should not just continue, gdb is fine when you come to fix it, but in regression you want it to stop dead. > > Last but not least: code residing in documentation should always > conform to codingstyle: DATA->data, etc. > > Will fix, I agree, in fact doxygen can take its example code from tests that are compiled and run, maybe we need to do it that way for the ARCH doc code snippets. > Kim > >
On Wed, Apr 16, 2014 at 1:42 PM, Kim Phillips <kim.phillips@freescale.com>wrote: > On Wed, 16 Apr 2014 12:42:58 -0400 > Mike Holmes <mike.holmes@linaro.org> wrote: > > > + > > +#if (offsetof(timer, DATA) != 4) > > +#error DATA must be at offset 4 in timer > > +#endif > > +@endcode > > IIRC, offsetof and friends aren't available in the c preprocessor - > [/me goes and checks] yep, this code fails to build: > > offsetofincpp.c:10:14: error: missing binary operator before token "(" > #if (offsetof(timer, DATA) != 4) > ^ > make: *** [offsetofincpp] Error 1 > > You have to #define your own macro in C. See http://linuxwell.com/2012/11/10/magical-container_of-macro/ for a detailed explanation of how this stuff works as compile time checks. The Linux kernel makes extensive use of this sort of thing. > fwiw, you can stop a compiler dead in its tracks by declaring > an array of negative length -> something a build-time ODP macro > might want to do if its condition is true, esp. if you want to avoid > the preprocessor and the limited ugliness it imposes on the code, as > shown above. > Yes, this typically done by a generated typedef, see http://stackoverflow.com/questions/807244/c-compiler-asserts-how-to-implementfor some useful discussion and examples. Bill
On Wed, Apr 16, 2014 at 08:00:22PM +0100, Bill Fischofer wrote: >> On Wed, Apr 16, 2014 at 1:42 PM, Kim Phillips <kim.phillips@freescale.com<mailto:kim.phillips@freescale.com>> wrote: >>> On Wed, 16 Apr 2014 12:42:58 -0400 >>> Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> wrote: >>> >>> + >>> +#if (offsetof(timer, DATA) != 4) >>> +#error DATA must be at offset 4 in timer >>> +#endif >>> +@endcode >> >> IIRC, offsetof and friends aren't available in the c preprocessor - >> [/me goes and checks] yep, this code fails to build: >> >> offsetofincpp.c:10:14: error: missing binary operator before token "(" >> #if (offsetof(timer, DATA) != 4) >> ^ >> make: *** [offsetofincpp] Error 1 >> >> You have to #define your own macro in C. See >> http://linuxwell.com/2012/11/10/magical-container_of-macro/ for a >> detailed explanation of how this stuff works as compile time checks. >> The Linux kernel makes extensive use of this sort of thing. >> >> fwiw, you can stop a compiler dead in its tracks by declaring >> an array of negative length -> something a build-time ODP macro >> might want to do if its condition is true, esp. if you want to avoid >> the preprocessor and the limited ugliness it imposes on the code, as >> shown above. >> >> Yes, this typically done by a generated typedef, see >> http://stackoverflow.com/questions/807244/c-compiler-asserts-how-to-implement >> for some useful discussion and examples. >> ODP_ASSERT is already defined in odp_debug.h as; #define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : -1] It should probably be renamed to ODP_STATIC_ASSERT. -- Stuart.
I thought I had seen that construct already in use. Thanks for the reference. Agree it should be ODP_STATIC_ASSERT because this sort of thing is now part of C11 in the new _Static_assert(cond,msg) construct. See http://www.robertgamble.net/2012/01/c11-static-assertions.html for details and examples. Since this is implemented in GCC 4.6 and higher we should probably have that macro expand appropriately based on compiler level. Bill On Thu, Apr 17, 2014 at 3:56 AM, Stuart Haslam <stuart.haslam@arm.com>wrote: > On Wed, Apr 16, 2014 at 08:00:22PM +0100, Bill Fischofer wrote: > >> On Wed, Apr 16, 2014 at 1:42 PM, Kim Phillips < > kim.phillips@freescale.com<mailto:kim.phillips@freescale.com>> wrote: > >>> On Wed, 16 Apr 2014 12:42:58 -0400 > >>> Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > wrote: > >>> > >>> + > >>> +#if (offsetof(timer, DATA) != 4) > >>> +#error DATA must be at offset 4 in timer > >>> +#endif > >>> +@endcode > >> > >> IIRC, offsetof and friends aren't available in the c preprocessor - > >> [/me goes and checks] yep, this code fails to build: > >> > >> offsetofincpp.c:10:14: error: missing binary operator before token "(" > >> #if (offsetof(timer, DATA) != 4) > >> ^ > >> make: *** [offsetofincpp] Error 1 > >> > >> You have to #define your own macro in C. See > >> http://linuxwell.com/2012/11/10/magical-container_of-macro/ for a > >> detailed explanation of how this stuff works as compile time checks. > >> The Linux kernel makes extensive use of this sort of thing. > >> > >> fwiw, you can stop a compiler dead in its tracks by declaring > >> an array of negative length -> something a build-time ODP macro > >> might want to do if its condition is true, esp. if you want to avoid > >> the preprocessor and the limited ugliness it imposes on the code, as > >> shown above. > >> > >> Yes, this typically done by a generated typedef, see > >> > http://stackoverflow.com/questions/807244/c-compiler-asserts-how-to-implement > >> for some useful discussion and examples. > >> > > ODP_ASSERT is already defined in odp_debug.h as; > > #define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : -1] > > It should probably be renamed to ODP_STATIC_ASSERT. > > -- > Stuart. > > >
Agree, I will incorporate the changes. On 17 April 2014 07:04, Bill Fischofer <bill.fischofer@linaro.org> wrote: > I thought I had seen that construct already in use. Thanks for the > reference. Agree it should be ODP_STATIC_ASSERT because this sort of thing > is now part of C11 in the new _Static_assert(cond,msg) construct. See > http://www.robertgamble.net/2012/01/c11-static-assertions.html for > details and examples. > > Since this is implemented in GCC 4.6 and higher we should probably have > that macro expand appropriately based on compiler level. > > Bill > > > On Thu, Apr 17, 2014 at 3:56 AM, Stuart Haslam <stuart.haslam@arm.com>wrote: > >> On Wed, Apr 16, 2014 at 08:00:22PM +0100, Bill Fischofer wrote: >> >> On Wed, Apr 16, 2014 at 1:42 PM, Kim Phillips < >> kim.phillips@freescale.com<mailto:kim.phillips@freescale.com>> wrote: >> >>> On Wed, 16 Apr 2014 12:42:58 -0400 >> >>> Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> >> wrote: >> >>> >> >>> + >> >>> +#if (offsetof(timer, DATA) != 4) >> >>> +#error DATA must be at offset 4 in timer >> >>> +#endif >> >>> +@endcode >> >> >> >> IIRC, offsetof and friends aren't available in the c preprocessor - >> >> [/me goes and checks] yep, this code fails to build: >> >> >> >> offsetofincpp.c:10:14: error: missing binary operator before token "(" >> >> #if (offsetof(timer, DATA) != 4) >> >> ^ >> >> make: *** [offsetofincpp] Error 1 >> >> >> >> You have to #define your own macro in C. See >> >> http://linuxwell.com/2012/11/10/magical-container_of-macro/ for a >> >> detailed explanation of how this stuff works as compile time checks. >> >> The Linux kernel makes extensive use of this sort of thing. >> >> >> >> fwiw, you can stop a compiler dead in its tracks by declaring >> >> an array of negative length -> something a build-time ODP macro >> >> might want to do if its condition is true, esp. if you want to avoid >> >> the preprocessor and the limited ugliness it imposes on the code, as >> >> shown above. >> >> >> >> Yes, this typically done by a generated typedef, see >> >> >> http://stackoverflow.com/questions/807244/c-compiler-asserts-how-to-implement >> >> for some useful discussion and examples. >> >> >> >> ODP_ASSERT is already defined in odp_debug.h as; >> >> #define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : -1] >> >> It should probably be renamed to ODP_STATIC_ASSERT. >> >> -- >> Stuart. >> >> >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
diff --git a/exception_handling.dox b/exception_handling.dox new file mode 100644 index 0000000..d99d5a0 --- /dev/null +++ b/exception_handling.dox @@ -0,0 +1,95 @@ +/* Copyright (c) 2013, Linaro Limited + * All rights reserved + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** +@page exception_handling Exception handling in the ODP API +@tableofcontents + +For the implimentation of the exception handling please see @ref odp_debug.h + +@section requirements Requirements +- Minimal overhead in a finished running system. +- Minimizing the propagation of an error from its point of origin +- Identifying what is a programming error +- Identifying a legitimate infield exception +- We only specify what happens inside the ODP library, not in a calling application + +There are two kinds of exceptional behaviour, +-# Run time exceptions, those that are unusual but foreseeable cases in a running system (out of memory) +-# Programming exceptions, those introduced as bugs (null pointers, out of bounds). + +@section run_time Run time exceptions +These are characterized by the following rules in order of importance +-# These must gracefully leave the system in a known stable state. +-# These checks must remain unconditionally in the code base. +-# These should return the error state to the caller. +-# They may emit an error message via \ref ODP_ERR which can be redefined or disabled. + +@subsection run_time_examples Examples +- Being "too late" to cancel a timer that's already popped, or exceeding some implementation-defined limit +- Backpressure due to resource limits (corner case that is error-prone) +- Checks for any condition that could arise in the field, e.g. running out of buffers or failure to allocate memory +@code + +if (unrecoverable_out_of_foos == 1) +{ + ODP_ERR("Completely unable to proceed, no foos available"); + tidy_op_for_exit(); + ... +} + +@endcode +@note ODP does not trap segfaults, it may not be checking for NULL pointers etc to improve the execution speed. The application should trap segfaults. + +@section programming_exceptions Programming exceptions +There are two classes of programming error +-# Compile time, these can be caught by compile time assertions in the preprocessor +-# Run Time, these are run time assertions + +@section compile_time Compile time programming exceptions +These have the following rules +-# Zero overhead at run time, they never need to be turned off (undefined) +-# Use @#error which will break the build, or @#warning which may not break the build unless -Werror is defined. +-# Can be done for any static evaluation case. + +@subsection compile_time_examples Examples +Checking size and alignment of a struct with offsetof + +@code +typedef struct timer timer; +struct timer +{ + uint8_t MODE; + uint32_t DATA; + uint32_t COUNT; +}; + + +#if (offsetof(timer, DATA) != 4) +#error DATA must be at offset 4 in timer +#endif +@endcode + +@section compile_run_time Run time programming exceptions +There are two rules +-# These must be capable of being turned off by defining -DODP_NO_DEBUG +-# They must use ODP_ASSERT so that the output may be redirected on systems without stderr. +-# ODP_ASSERT will call abort() as its final operation. + +@note ODP_ASSERT is defined to make it easier to redirect output from stderr. For example +an in memory text buffer may be in use if stderr has no meaning on a bare metal implimentation + +@subsection compile_run_time_examples Examples +Checks that the API function arguments are within the permitted value range (e.g. handle validation + +@code +void odp_foo(char *pointer) +{ + ODP_ASSERT(pointer != NULL); + … +} +@endcode +*/
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- exception_handling.dox | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 exception_handling.dox