| Message ID | 1412698530-2999-1-git-send-email-bala.manoharan@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 7 October 2014 09:15, Balasubramanian Manoharan <bala.manoharan@linaro.org> wrote: > This patch provides ODP macro for unimplemented function > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > --- > 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. 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. Thanks, Victor > + > +/** > * Print output to stderr (file, line and function), > * then abort. > */ > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> wrote: > On 7 October 2014 09:15, Balasubramanian Manoharan > <bala.manoharan@linaro.org> wrote: > > This patch provides ODP macro for unimplemented function > > > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > --- > > 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 > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
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 <mike.holmes@linaro.org> wrote: > > > On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> > wrote: > >> On 7 October 2014 09:15, Balasubramanian Manoharan >> <bala.manoharan@linaro.org> wrote: >> > This patch provides ODP macro for unimplemented function >> > >> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> > --- >> > 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 >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
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 <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> > wrote: > >> >> >> On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> >> wrote: >> >>> On 7 October 2014 09:15, Balasubramanian Manoharan >>> <bala.manoharan@linaro.org> wrote: >>> > This patch provides ODP macro for unimplemented function >>> > >>> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> > --- >>> > 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 >>> > lng-odp@lists.linaro.org >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 7 October 2014 13:41, Bala Manoharan <bala.manoharan@linaro.org> wrote: > 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 > Unless you change the other existing functions that call fprintf and printf to ODP_LOG I think the style should be consistent with what is there now, this patch is starting to suffer feature creep with new macros etc. If we make it simple now I think it will be simple later when we just make a wholesale change to logging, rather than a dribble of changes that leave us performing the task two ways for a number of weeks or worse months. > > 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? > I think we need a new location for internally shared files. We have platform/linux-generic/include/api should we add platform/linux-generic/include/api-internal and place the new file in there ? That allows the current overridden internal platform specifics to override either location in platform/linux-generic/include as they do now. > > Regards, > Bala > > On 7 October 2014 22:55, Bill Fischofer <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> >> wrote: >> >>> >>> >>> On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> >>> wrote: >>> >>>> On 7 October 2014 09:15, Balasubramanian Manoharan >>>> <bala.manoharan@linaro.org> wrote: >>>> > This patch provides ODP macro for unimplemented function >>>> > >>>> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>>> > --- >>>> > 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 >>>> > lng-odp@lists.linaro.org >>>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Sr Technical Manager >>> LNG - ODP >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
On 7 October 2014 18:45, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> > wrote: > >> On 7 October 2014 09:15, Balasubramanian Manoharan >> <bala.manoharan@linaro.org> wrote: >> > This patch provides ODP macro for unimplemented function >> > >> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> > --- >> > 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. > > You all have the source code, you don't need me. -- Ola
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… -Petri From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext Bala Manoharan Sent: Tuesday, October 07, 2014 8:41 PM To: Bill Fischofer Cc: lng-odp@lists.linaro.org 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 <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>> 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 <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> wrote: On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org<mailto:victor.kamensky@linaro.org>> wrote: On 7 October 2014 09:15, Balasubramanian Manoharan <bala.manoharan@linaro.org<mailto:bala.manoharan@linaro.org>> wrote: > This patch provides ODP macro for unimplemented function > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org<mailto:bala.manoharan@linaro.org>> > --- > 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 > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ 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 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp
On Wed, Oct 08, 2014 at 10:40:58AM +0100, Savolainen, Petri (NSN - FI/Espoo) 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… > > -Petri > It looks like odp_debug.h should be moved into up into include/ at some point. The only thing in there at the minute that is being used by applications are the ODP_DBG/ODP_ERR/ODP_ABORT macros and they're due to be replaced (and should've been treated as internal from the beginning anyway). -- Stuart.
On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> 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:* lng-odp-bounces@lists.linaro.org [mailto: > lng-odp-bounces@lists.linaro.org] *On Behalf Of *ext Bala Manoharan > *Sent:* Tuesday, October 07, 2014 8:41 PM > *To:* Bill Fischofer > *Cc:* lng-odp@lists.linaro.org > *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 <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> > wrote: > > > > > > On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> > wrote: > > On 7 October 2014 09:15, Balasubramanian Manoharan > <bala.manoharan@linaro.org> wrote: > > This patch provides ODP macro for unimplemented function > > > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > --- > > 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 > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
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 <mike.holmes@linaro.org> wrote: > > > On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> 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:* lng-odp-bounces@lists.linaro.org [mailto: >> lng-odp-bounces@lists.linaro.org] *On Behalf Of *ext Bala Manoharan >> *Sent:* Tuesday, October 07, 2014 8:41 PM >> *To:* Bill Fischofer >> *Cc:* lng-odp@lists.linaro.org >> *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 <bill.fischofer@linaro.org> >> 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 <mike.holmes@linaro.org> >> wrote: >> >> >> >> >> >> On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> >> wrote: >> >> On 7 October 2014 09:15, Balasubramanian Manoharan >> <bala.manoharan@linaro.org> wrote: >> > This patch provides ODP macro for unimplemented function >> > >> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> > --- >> > 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 >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> >> -- >> >> *Mike Holmes* >> >> Linaro Sr Technical Manager >> >> LNG - ODP >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP >
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 <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> > wrote: > >> >> >> On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) < >> petri.savolainen@nsn.com> 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:* lng-odp-bounces@lists.linaro.org [mailto: >>> lng-odp-bounces@lists.linaro.org] *On Behalf Of *ext Bala Manoharan >>> *Sent:* Tuesday, October 07, 2014 8:41 PM >>> *To:* Bill Fischofer >>> *Cc:* lng-odp@lists.linaro.org >>> *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 <bill.fischofer@linaro.org> >>> 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 <mike.holmes@linaro.org> >>> wrote: >>> >>> >>> >>> >>> >>> On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> >>> wrote: >>> >>> On 7 October 2014 09:15, Balasubramanian Manoharan >>> <bala.manoharan@linaro.org> wrote: >>> > This patch provides ODP macro for unimplemented function >>> > >>> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> > --- >>> > 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 >>> > lng-odp@lists.linaro.org >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> >>> >>> -- >>> >>> *Mike Holmes* >>> >>> Linaro Sr Technical Manager >>> >>> LNG - ODP >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> > >
“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:bala.manoharan@linaro.org] Sent: Thursday, October 09, 2014 8:03 AM To: Bill Fischofer Cc: Mike Holmes; Savolainen, Petri (NSN - FI/Espoo); lng-odp@lists.linaro.org 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 <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>> 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 <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> wrote: On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> 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: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org>] On Behalf Of ext Bala Manoharan Sent: Tuesday, October 07, 2014 8:41 PM To: Bill Fischofer Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> 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 <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>> 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 <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> wrote: On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org<mailto:victor.kamensky@linaro.org>> wrote: On 7 October 2014 09:15, Balasubramanian Manoharan <bala.manoharan@linaro.org<mailto:bala.manoharan@linaro.org>> wrote: > This patch provides ODP macro for unimplemented function > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org<mailto:bala.manoharan@linaro.org>> > --- > 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 > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ 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 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ 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 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> 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:bala.manoharan@linaro.org] > *Sent:* Thursday, October 09, 2014 8:03 AM > *To:* Bill Fischofer > *Cc:* Mike Holmes; Savolainen, Petri (NSN - FI/Espoo); > lng-odp@lists.linaro.org > > *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 <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> > wrote: > > > > > > On 8 October 2014 05:40, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> 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:* lng-odp-bounces@lists.linaro.org [mailto: > lng-odp-bounces@lists.linaro.org] *On Behalf Of *ext Bala Manoharan > *Sent:* Tuesday, October 07, 2014 8:41 PM > *To:* Bill Fischofer > *Cc:* lng-odp@lists.linaro.org > *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 <bill.fischofer@linaro.org> 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 <mike.holmes@linaro.org> > wrote: > > > > > > On 7 October 2014 12:27, Victor Kamensky <victor.kamensky@linaro.org> > wrote: > > On 7 October 2014 09:15, Balasubramanian Manoharan > <bala.manoharan@linaro.org> wrote: > > This patch provides ODP macro for unimplemented function > > > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > --- > > 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 > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Sr Technical Manager > > LNG - ODP > > > > >
From: ext Mike Holmes [mailto:mike.holmes@linaro.org]
Sent: Friday, October 10, 2014 2:21 AM
To: Savolainen, Petri (NSN - FI/Espoo)
Cc: ext Bala Manoharan; Bill Fischofer; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for unimplemented function
On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> 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
Admit that _internal is not optimal post-fix, but it highlights that these files are _not_ API. Those are part of the implementation of an API. I think we must keep API and non-API files strictly separated. So that API file names are not reused anywhere else in the implementation. Some better pre or post fix could be used.
-Petri
Looks like something of a deeper move does not fall out that easily, so I also vote for linux-generic/include/odp_debug_internal.h to get this in. On 10 October 2014 04:20, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > > From: ext Mike Holmes [mailto:mike.holmes@linaro.org] > Sent: Friday, October 10, 2014 2:21 AM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: ext Bala Manoharan; Bill Fischofer; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for unimplemented > function > > > > On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> 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 > > > Admit that _internal is not optimal post-fix, but it highlights that these > files are _not_ API. Those are part of the implementation of an API. I > think we must keep API and non-API files strictly separated. So that API > file names are not reused anywhere else in the implementation. Some better > pre or post fix could be used. > > -Petri > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Hi All, If no one has any objections I will move ODP_UNIMPLEMENTED/ODP_ERR and ODP_ABORT into odp_debug_internal.h file and provide a new patch. I will also remove "fprintf" from individual macros and implement a macro ODP_LOG? Pls let me know if there are any concerns for the above suggestion. Regards, Bala On 10 October 2014 16:59, Mike Holmes <mike.holmes@linaro.org> wrote: > Looks like something of a deeper move does not fall out that easily, so I > also vote for linux-generic/include/odp_debug_internal.h to get this in. > > On 10 October 2014 04:20, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > >> >> >> From: ext Mike Holmes [mailto:mike.holmes@linaro.org] >> Sent: Friday, October 10, 2014 2:21 AM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: ext Bala Manoharan; Bill Fischofer; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for unimplemented >> function >> >> >> >> On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) < >> petri.savolainen@nsn.com> 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 >> >> >> Admit that _internal is not optimal post-fix, but it highlights that >> these files are _not_ API. Those are part of the implementation of an API. >> I think we must keep API and non-API files strictly separated. So that API >> file names are not reused anywhere else in the implementation. Some better >> pre or post fix could be used. >> >> -Petri >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 10/13/2014 12:22 PM, Bala Manoharan wrote: > Hi All, > > If no one has any objections I will move ODP_UNIMPLEMENTED/ODP_ERR and > ODP_ABORT into odp_debug_internal.h file and provide a new patch. > I will also remove "fprintf" from individual macros and implement a > macro ODP_LOG? > > Pls let me know if there are any concerns for the above suggestion. > > Regards, > Bala Yes, I think it's reasonable. Nothing stops us in future to put that to global api. So let's do it for linux-generic and if there will not be changes for other applications we can move that to global include. Maxim. > > On 10 October 2014 16:59, Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > Looks like something of a deeper move does not fall out that > easily, so I also vote for > linux-generic/include/odp_debug_internal.h to get this in. > > On 10 October 2014 04:20, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com <mailto:petri.savolainen@nsn.com>> wrote: > > > > From: ext Mike Holmes [mailto:mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>] > Sent: Friday, October 10, 2014 2:21 AM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: ext Bala Manoharan; Bill Fischofer; > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for > unimplemented function > > > > On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com <mailto:petri.savolainen@nsn.com>> > 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 > > > Admit that _internal is not optimal post-fix, but it > highlights that these files are _not_ API. Those are part of > the implementation of an API. I think we must keep API and > non-API files strictly separated. So that API file names are > not reused anywhere else in the implementation. Some better > pre or post fix could be used. > > -Petri > > > _______________________________________________ > 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 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
Gets my vote On 13 October 2014 06:38, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 10/13/2014 12:22 PM, Bala Manoharan wrote: > >> Hi All, >> >> If no one has any objections I will move ODP_UNIMPLEMENTED/ODP_ERR and >> ODP_ABORT into odp_debug_internal.h file and provide a new patch. >> I will also remove "fprintf" from individual macros and implement a macro >> ODP_LOG? >> >> Pls let me know if there are any concerns for the above suggestion. >> >> Regards, >> Bala >> > > Yes, I think it's reasonable. Nothing stops us in future to put that to > global api. So let's do it for linux-generic and if there will not be > changes for other applications we can move that to global include. > > Maxim. > > > >> On 10 October 2014 16:59, Mike Holmes <mike.holmes@linaro.org <mailto: >> mike.holmes@linaro.org>> wrote: >> >> Looks like something of a deeper move does not fall out that >> easily, so I also vote for >> linux-generic/include/odp_debug_internal.h to get this in. >> >> On 10 October 2014 04:20, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolainen@nsn.com <mailto:petri.savolainen@nsn.com>> wrote: >> >> >> >> From: ext Mike Holmes [mailto:mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>] >> Sent: Friday, October 10, 2014 2:21 AM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: ext Bala Manoharan; Bill Fischofer; >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> Subject: Re: [lng-odp] [ODP/PATCH 2/2 v1] ODP Macro for >> unimplemented function >> >> >> >> On 9 October 2014 02:43, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolainen@nsn.com <mailto:petri.savolainen@nsn.com>> >> >> 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 >> >> >> Admit that _internal is not optimal post-fix, but it >> highlights that these files are _not_ API. Those are part of >> the implementation of an API. I think we must keep API and >> non-API files strictly separated. So that API file names are >> not reused anywhere else in the implementation. Some better >> pre or post fix could be used. >> >> -Petri >> >> >> _______________________________________________ >> 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 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > 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 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__); + +/** * Print output to stderr (file, line and function), * then abort. */
This patch provides ODP macro for unimplemented function Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> --- platform/linux-generic/include/api/odp_debug.h | 7 +++++++ 1 file changed, 7 insertions(+)