| Message ID | 1416413564-14626-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
Can you work the weak linkage replacement for ODP_ASSERT into this patch so that the application can redirect calls to abort. I posed an ODP_WEAK patch yesterday. The unit tests need to be able to redirect the abort right now so this would be very valuable immediately. On 19 November 2014 11:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Fix macro logic. Application has to be terminated in any case. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/include/api/odp_debug.h > b/platform/linux-generic/include/api/odp_debug.h > index c9b2edd..1cbcb61 100644 > --- a/platform/linux-generic/include/api/odp_debug.h > +++ b/platform/linux-generic/include/api/odp_debug.h > @@ -57,10 +57,13 @@ extern "C" { > /** > * Runtime assertion-macro - aborts if 'cond' is false. > */ > -#define ODP_ASSERT(cond, msg) \ > - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ > - ODP_ERR("%s\n", msg); \ > - abort(); } \ > +#define ODP_ASSERT(cond, msg) \ > + do { \ > + if (!(cond)) { \ > + if (ODP_DEBUG == 1) \ > + ODP_ERR("%s\n", msg); \ > + abort(); \ > + } \ > } while (0) > > /** > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 11/19/2014 07:28 PM, Mike Holmes wrote: > Can you work the weak linkage replacement for ODP_ASSERT into this > patch so that the application can redirect calls to abort. > I posed an ODP_WEAK patch yesterday. __weak__ is symbol attribute (function, variable) not macros. Do you want to make ODP_ASSERT function? Maxim. > > The unit tests need to be able to redirect the abort right now so this > would be very valuable immediately. > > On 19 November 2014 11:12, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Fix macro logic. Application has to be terminated in any case. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > --- > platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/include/api/odp_debug.h > b/platform/linux-generic/include/api/odp_debug.h > index c9b2edd..1cbcb61 100644 > --- a/platform/linux-generic/include/api/odp_debug.h > +++ b/platform/linux-generic/include/api/odp_debug.h > @@ -57,10 +57,13 @@ extern "C" { > /** > * Runtime assertion-macro - aborts if 'cond' is false. > */ > -#define ODP_ASSERT(cond, msg) \ > - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ > - ODP_ERR("%s\n", msg); \ > - abort(); } \ > +#define ODP_ASSERT(cond, msg) \ > + do { \ > + if (!(cond)) { \ > + if (ODP_DEBUG == 1) \ > + ODP_ERR("%s\n", msg); \ > + abort(); \ > + } \ > } while (0) > > /** > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP
On 19 November 2014 11:46, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 11/19/2014 07:28 PM, Mike Holmes wrote: > >> Can you work the weak linkage replacement for ODP_ASSERT into this patch >> so that the application can redirect calls to abort. >> I posed an ODP_WEAK patch yesterday. >> > > __weak__ is symbol attribute (function, variable) not macros. Do you want > to make ODP_ASSERT function? > Yes - the plan of record was that we would do this rather than pass pointers to application functions in odp init global init We need that ODP_ASSERT to call the current default functionality as a function and add to the new default implementation ODP_WEAK > > Maxim. > > >> The unit tests need to be able to redirect the abort right now so this >> would be very valuable immediately. >> >> On 19 November 2014 11:12, Maxim Uvarov <maxim.uvarov@linaro.org <mailto: >> maxim.uvarov@linaro.org>> wrote: >> >> Fix macro logic. Application has to be terminated in any case. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> >> >> --- >> platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/platform/linux-generic/include/api/odp_debug.h >> b/platform/linux-generic/include/api/odp_debug.h >> index c9b2edd..1cbcb61 100644 >> --- a/platform/linux-generic/include/api/odp_debug.h >> +++ b/platform/linux-generic/include/api/odp_debug.h >> @@ -57,10 +57,13 @@ extern "C" { >> /** >> * Runtime assertion-macro - aborts if 'cond' is false. >> */ >> -#define ODP_ASSERT(cond, msg) \ >> - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ >> - ODP_ERR("%s\n", msg); \ >> - abort(); } \ >> +#define ODP_ASSERT(cond, msg) \ >> + do { \ >> + if (!(cond)) { \ >> + if (ODP_DEBUG == 1) \ >> + ODP_ERR("%s\n", msg); \ >> + abort(); \ >> + } \ >> } while (0) >> >> /** >> -- >> 1.8.5.1.163.gd7aced9 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> > >
On 11/19/2014 06:12 PM, Maxim Uvarov wrote: > Fix macro logic. Application has to be terminated in any case. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h > index c9b2edd..1cbcb61 100644 > --- a/platform/linux-generic/include/api/odp_debug.h > +++ b/platform/linux-generic/include/api/odp_debug.h > @@ -57,10 +57,13 @@ extern "C" { > /** > * Runtime assertion-macro - aborts if 'cond' is false. > */ > -#define ODP_ASSERT(cond, msg) \ > - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ > - ODP_ERR("%s\n", msg); \ > - abort(); } \ > +#define ODP_ASSERT(cond, msg) \ > + do { \ > + if (!(cond)) { \ > + if (ODP_DEBUG == 1) \ > + ODP_ERR("%s\n", msg); \ > + abort(); \ > + } \ > } while (0) > > /** > Current specification says that ODP_ASSERT can be disabled if not needed. So it can be used in a fastpath to catch programming errors and then disabled for release build. If you want to have persistent asserts then let's make two macros: - ODP_ASSERT() - persistent assert - ODP_DEBUG_ASSERT() - assert enabled only in debug build. BTW is this a part of public API? Will applications use it? I seems not.
On 11/20/2014 11:56 AM, Taras Kondratiuk wrote: > On 11/19/2014 06:12 PM, Maxim Uvarov wrote: >> Fix macro logic. Application has to be terminated in any case. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/platform/linux-generic/include/api/odp_debug.h >> b/platform/linux-generic/include/api/odp_debug.h >> index c9b2edd..1cbcb61 100644 >> --- a/platform/linux-generic/include/api/odp_debug.h >> +++ b/platform/linux-generic/include/api/odp_debug.h >> @@ -57,10 +57,13 @@ extern "C" { >> /** >> * Runtime assertion-macro - aborts if 'cond' is false. >> */ >> -#define ODP_ASSERT(cond, msg) \ >> - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ >> - ODP_ERR("%s\n", msg); \ >> - abort(); } \ >> +#define ODP_ASSERT(cond, msg) \ >> + do { \ >> + if (!(cond)) { \ >> + if (ODP_DEBUG == 1) \ >> + ODP_ERR("%s\n", msg); \ >> + abort(); \ >> + } \ >> } while (0) >> >> /** >> > > Current specification says that ODP_ASSERT can be disabled if not > needed. So it can be used in a fastpath to catch programming errors > and then disabled for release build. > > If you want to have persistent asserts then let's make two macros: > - ODP_ASSERT() - persistent assert > - ODP_DEBUG_ASSERT() - assert enabled only in debug build. > > BTW is this a part of public API? Will applications use it? I seems > not. If it's in include/api/ header that it's public api. So that apps can use it. But we do not use it in odp.git. I agree that we need rename original to ODP_DEBUG_ASSERT(). And new version to ODP_ASSERT() because ODP_ASSERT() was really confusing for me. Trap only in debug case. Maxim.
On 20 November 2014 04:18, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 11/20/2014 11:56 AM, Taras Kondratiuk wrote: > >> On 11/19/2014 06:12 PM, Maxim Uvarov wrote: >> >>> Fix macro logic. Application has to be terminated in any case. >>> >>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >>> --- >>> platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/api/odp_debug.h >>> b/platform/linux-generic/include/api/odp_debug.h >>> index c9b2edd..1cbcb61 100644 >>> --- a/platform/linux-generic/include/api/odp_debug.h >>> +++ b/platform/linux-generic/include/api/odp_debug.h >>> @@ -57,10 +57,13 @@ extern "C" { >>> /** >>> * Runtime assertion-macro - aborts if 'cond' is false. >>> */ >>> -#define ODP_ASSERT(cond, msg) \ >>> - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ >>> - ODP_ERR("%s\n", msg); \ >>> - abort(); } \ >>> +#define ODP_ASSERT(cond, msg) \ >>> + do { \ >>> + if (!(cond)) { \ >>> + if (ODP_DEBUG == 1) \ >>> + ODP_ERR("%s\n", msg); \ >>> + abort(); \ >>> + } \ >>> } while (0) >>> >>> /** >>> >>> >> Current specification says that ODP_ASSERT can be disabled if not >> needed. So it can be used in a fastpath to catch programming errors >> and then disabled for release build. >> >> If you want to have persistent asserts then let's make two macros: >> - ODP_ASSERT() - persistent assert >> - ODP_DEBUG_ASSERT() - assert enabled only in debug build. >> >> BTW is this a part of public API? Will applications use it? I seems >> not. >> > If it's in include/api/ header that it's public api. So that apps can use > it. But we do not use it in odp.git. > Applications should not use it, it should be internal to the implementation > I agree that we need rename original to ODP_DEBUG_ASSERT(). And new > version to ODP_ASSERT() > because ODP_ASSERT() was really confusing for me. Trap only in debug case. Why do we need a persistent ODP_ASSERT ? I could see renaming it ODP_RUN_ASSERT to match ODP_STATIC_ASSERT, not sure of a need for any other renaming or additional asserts types. It has to be possible to remove it entirely for performance reasons, but when present it has to be replaceable. What is the use case I am forgetting ? > > > Maxim. > > > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h index c9b2edd..1cbcb61 100644 --- a/platform/linux-generic/include/api/odp_debug.h +++ b/platform/linux-generic/include/api/odp_debug.h @@ -57,10 +57,13 @@ extern "C" { /** * Runtime assertion-macro - aborts if 'cond' is false. */ -#define ODP_ASSERT(cond, msg) \ - do { if ((ODP_DEBUG == 1) && (!(cond))) { \ - ODP_ERR("%s\n", msg); \ - abort(); } \ +#define ODP_ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + if (ODP_DEBUG == 1) \ + ODP_ERR("%s\n", msg); \ + abort(); \ + } \ } while (0) /**
Fix macro logic. Application has to be terminated in any case. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/include/api/odp_debug.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)