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]> 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]> > wrote: > >> >> >> On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) < >> [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]] *On Behalf Of *ext Bala Manoharan >>> *Sent:* Tuesday, October 07, 2014 8:41 PM >>> *To:* Bill Fischofer >>> *Cc:* [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]> >>> 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]> >>> wrote: >>> >>> >>> >>> >>> >>> On 7 October 2014 12:27, Victor Kamensky <[email protected]> >>> wrote: >>> >>> On 7 October 2014 09:15, Balasubramanian Manoharan >>> <[email protected]> wrote: >>> > This patch provides ODP macro for unimplemented function >>> > >>> > Signed-off-by: Balasubramanian Manoharan <[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] >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> _______________________________________________ >>> lng-odp mailing list >>> [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 >>> >>> >>> >>> >>> _______________________________________________ >>> 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 >>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
