I think having the various override functions be consistently exposed as weak externals is cleaner and more extensible than having to pass an ever-growing list of functions to odp_global_init(). It will also make it possible to develop extension frameworks that provide these overrides independent of any ODP application that makes use of the extensions.
On Fri, Nov 21, 2014 at 4:54 PM, Mike Holmes <[email protected]> wrote: > > > On 21 November 2014 04:11, Taras Kondratiuk <[email protected]> > wrote: > >> On 11/20/2014 08:42 PM, Maxim Uvarov wrote: >> >>> On 11/20/2014 09:36 PM, Maxim Uvarov wrote: >>> >>>> On 11/20/2014 08:27 PM, Taras Kondratiuk wrote: >>>> >>>>> ODP application may want to override default ODP logging behaviour >>>>> and use custom logging function. Add a weak odp_override_log() function >>>>> for this purpose instead of default fprintf(). >>>>> >>>>> Signed-off-by: Taras Kondratiuk <[email protected]> >>>>> --- >>>>> platform/linux-generic/include/api/odp_debug.h | 24 >>>>> ++++++++++++++---------- >>>>> platform/linux-generic/odp_init.c | 14 ++++++++++++++ >>>>> 2 files changed, 28 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/platform/linux-generic/include/api/odp_debug.h >>>>> b/platform/linux-generic/include/api/odp_debug.h >>>>> index c9b2edd..be73318 100644 >>>>> --- a/platform/linux-generic/include/api/odp_debug.h >>>>> +++ b/platform/linux-generic/include/api/odp_debug.h >>>>> @@ -14,6 +14,7 @@ >>>>> #include <stdio.h> >>>>> #include <stdlib.h> >>>>> +#include <stdarg.h> >>>>> #ifdef __cplusplus >>>>> extern "C" { >>>>> @@ -79,6 +80,9 @@ typedef enum odp_log_level { >>>>> ODP_LOG_ABORT >>>>> } odp_log_level_e; >>>>> + >>>>> +extern int odp_override_log(odp_log_level_e level, const char *fmt, >>>>> ...); >>>>> + >>>>> /** >>>>> * ODP default LOG macro. >>>>> */ >>>>> @@ -86,45 +90,45 @@ typedef enum odp_log_level { >>>>> do { \ >>>>> switch (level) { \ >>>>> case ODP_LOG_ERR: \ >>>>> - fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \ >>>>> + odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \ >>>>> __LINE__, __func__, ##__VA_ARGS__); \ >>>>> break; \ >>>>> case ODP_LOG_DBG: \ >>>>> if (ODP_DEBUG_PRINT == 1) \ >>>>> - fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \ >>>>> - __LINE__, __func__, ##__VA_ARGS__); \ >>>>> + odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \ >>>>> + __LINE__, __func__, ##__VA_ARGS__); \ >>>>> break; \ >>>>> case ODP_LOG_ABORT: \ >>>>> - fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ >>>>> + odp_override_log(level, "%s:%d:%s(): " fmt, __FILE__, \ >>>>> __LINE__, __func__, ##__VA_ARGS__); \ >>>>> abort(); \ >>>>> break; \ >>>>> case ODP_LOG_UNIMPLEMENTED: \ >>>>> - fprintf(stderr, \ >>>>> + odp_override_log(level, \ >>>>> "%s:%d:The function %s() is not implemented\n" \ >>>>> fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__); \ >>>>> break; \ >>>>> default: \ >>>>> - fprintf(stderr, "Unknown LOG level"); \ >>>>> + odp_override_log(level, "Unknown LOG level"); \ >>>>> break;\ >>>>> } \ >>>>> } while (0) >>>>> /** >>>>> - * Debug printing macro, which prints output when DEBUG flag is set. >>>>> + * Log debug message if DEBUG flag is set. >>>>> */ >>>>> #define ODP_DBG(fmt, ...) \ >>>>> ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__) >>>>> /** >>>>> - * Print output to stderr (file, line and function). >>>>> + * Log error message. >>>>> */ >>>>> #define ODP_ERR(fmt, ...) \ >>>>> ODP_LOG(ODP_LOG_ERR, fmt, ##__VA_ARGS__) >>>>> /** >>>>> - * Print output to stderr (file, line and function), >>>>> - * then abort. >>>>> + * Log abort message and then stop execution (by default call >>>>> abort()). >>>>> + * This function should not return. >>>>> */ >>>>> #define ODP_ABORT(fmt, ...) \ >>>>> ODP_LOG(ODP_LOG_ABORT, fmt, ##__VA_ARGS__) >>>>> diff --git a/platform/linux-generic/odp_init.c >>>>> b/platform/linux-generic/odp_init.c >>>>> index 672b3d6..7fbfe36 100644 >>>>> --- a/platform/linux-generic/odp_init.c >>>>> +++ b/platform/linux-generic/odp_init.c >>>>> @@ -8,6 +8,7 @@ >>>>> #include <odp_internal.h> >>>>> #include <odp_debug.h> >>>>> #include <odp_debug_internal.h> >>>>> +#include <odp_hints.h> >>>>> int odp_init_global(odp_init_t *params ODP_UNUSED, >>>>> @@ -89,3 +90,16 @@ int odp_term_local(void) >>>>> ODP_UNIMPLEMENTED(); >>>>> return 0; >>>>> } >>>>> + >>>>> +ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level, >>>>> + const char *fmt, ...) >>>>> +{ >>>>> + va_list args; >>>>> + int r; >>>>> + (void)level; >>>>> + va_start(args, fmt); >>>>> + r = vfprintf(stdout, fmt, args); >>>>> >>>> >>>> don't like stdout here. It might be stderr or file descriptor. >>>> >>>> Maxim. >>>> >>>> Ah, it's weak function. And you provided level also. Withdraw my note. >>> >> >> That actually was stderr first. But for testing I've changed it to >> stdout and forget to change it back. Will update in the next version. >> >> Is odp_init.c a good place for this function or it should be in a >> separate file? Like odp_log.c or odp_weaks.c > > > I favor odp_weak.c this is not an init function so I don't think > odp_init.c is the right place. odp_log might be good but we know we will be > adding a weak symbol for odp_abort, so we can group them I think. > > >> >> >> >>> Maxim. >>> >>> + va_end(args); >>>>> + >>>>> + return r; >>>>> +} >>>>> >>>> >>>> >>> >>> _______________________________________________ >>> 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
