On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) < [email protected]> wrote:
> “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 > Ok so drop api, I could see that making sense. it still feels redundant to have a long list of files all adding "_internal.h" mixed in with those that do not have it, if they are different in some way, why not capture that fact once in another directory and remove the repeated "_internal" text from each file name ? Currently in linux-generic/include/ odp_packet_internal.h odp_queue_internal.h odp_buffer_internal.h odp_packet_io_internal.h odp_schedule_internal.h odp_buffer_pool_internal.h odp_spin_internal.h odp_timer_internal.h odp_crypto_internal.h AND odp_packet_io_queue.h odp_packet_netmap.h odp_internal.h odp_packet_socket.h Mike > > > > > -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]> 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 > > > > > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
