“api-internal” does not make sense. There’s no “internal application programming interface”. If ABORT/ERR/etc are removed from the API, then those are just part of the implementation. It’s not very productive to try to standardize the implementation. We can place those macros in linux-generic so that those are easy find, but each implementation re-uses those at own risk.
If those are removed from odp_debug.h, a natural place to put those would be linux-generic/include/odp_debug_internal.h -Petri From: ext Bala Manoharan [mailto:[email protected]] Sent: Thursday, October 09, 2014 8:03 AM To: Bill Fischofer Cc: Mike Holmes; Savolainen, Petri (NSN - FI/Espoo); [email protected] Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for unimplemented function Do we just move ODP_ABORT/ERR/UNIMPLEMENTED all these macros into a separate folder inside platform /include/api-internal/odp_internal.h or does it make sense to move the whole file odp_debug.h file into this folder? Regards, Bala On 8 October 2014 21:28, Bill Fischofer <[email protected]<mailto:[email protected]>> wrote: include/api-internal makes sense. I agree having one of these per platform is duplicative and unnecessary. On Wed, Oct 8, 2014 at 7:23 AM, Mike Holmes <[email protected]<mailto:[email protected]>> wrote: On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) <[email protected]<mailto:[email protected]>> wrote: Certainly this does not belong to the API. Application would not ever call it. In linux-generic you can put it e.g. in include/odp_internal.h… Do we need a directory for internal files that are likely common to all implementations, rather than have separate copies per platform, introducing instead include/api-internal to move these items into? -Petri From: [email protected]<mailto:[email protected]> [mailto:[email protected]<mailto:[email protected]>] On Behalf Of ext Bala Manoharan Sent: Tuesday, October 07, 2014 8:41 PM To: Bill Fischofer Cc: [email protected]<mailto:[email protected]> Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for unimplemented function Agreed. I kept it as fprintf to be compliant with the existing syntax of ODP_ERR/ODP_ABORT But I can change this macro for now and further changes can be done after logger gets implemented Regarding the placing of this macro. I was thinking of a way in which this could be replaced by different platform specific implementations. IMO we can move the macro's once logger gets implemented. Any comments regarding the location of this macro? Regards, Bala On 7 October 2014 22:55, Bill Fischofer <[email protected]<mailto:[email protected]>> wrote: Agree this should be using ODP_LOG(). Include a prototype ODP_LOG() that does the fprintf() until the real logger is present (it's unimplemented). That way only that needs to be changed when the real logger shows up. On Tue, Oct 7, 2014 at 11:45 AM, Mike Holmes <[email protected]<mailto:[email protected]>> wrote: On 7 October 2014 12:27, Victor Kamensky <[email protected]<mailto:[email protected]>> wrote: On 7 October 2014 09:15, Balasubramanian Manoharan <[email protected]<mailto:[email protected]>> wrote: > This patch provides ODP macro for unimplemented function > > Signed-off-by: Balasubramanian Manoharan > <[email protected]<mailto:[email protected]>> > --- > platform/linux-generic/include/api/odp_debug.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_debug.h > b/platform/linux-generic/include/api/odp_debug.h > index 344b0a9..bb67cef 100644 > --- a/platform/linux-generic/include/api/odp_debug.h > +++ b/platform/linux-generic/include/api/odp_debug.h > @@ -82,6 +82,13 @@ do { fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ > } while (0) > > /** > + * This macro is used to indicate when a given function is not implemented > + */ > +#define ODP_UNIMPLEMENTED() \ > + fprintf(stderr, "%s:%d:The function %s() is yet to be implemented", \ > + __FILE__, __LINE__, __func__); Please don't use unsolicited fprintf, instead use proper logging call. That could be redirected in the future. We are trying to get Olas logging proposal in place, but we are not there yet, I think for this patch it is ok. I expect logging to get in before 1.0. Also it is very strange to see such definition in *api* header file. If it is utility macro to help implement standard message from unimplemented functions it should be placed properly - in some header file that only implementations can see. Agree, discussions this morning also highlighted a number of other items that might need to move. Items that can be possibly be common between platforms but are support for the implementation rather than part of the API. Thanks, Victor > + > +/** > * Print output to stderr (file, line and function), > * then abort. > */ > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > [email protected]<mailto:[email protected]> > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP _______________________________________________ lng-odp mailing list [email protected]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
