On 7 October 2014 13:41, Bala Manoharan <[email protected]> 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 <[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 >> >> > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
