On 2 December 2014 at 06:27, Taras Kondratiuk <[email protected]> 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/Makefile.am | 3 +- > platform/linux-generic/include/api/odp_debug.h | 41 > ++++++++++++++++-------- > platform/linux-generic/odp_weak.c | 23 +++++++++++++ > 3 files changed, 53 insertions(+), 14 deletions(-) > create mode 100644 platform/linux-generic/odp_weak.c > > diff --git a/platform/linux-generic/Makefile.am > b/platform/linux-generic/Makefile.am > index e709700..cc78de3 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -75,4 +75,5 @@ __LIB__libodp_la_SOURCES = \ > odp_thread.c \ > odp_ticketlock.c \ > odp_time.c \ > - odp_timer.c > + odp_timer.c \ > + odp_weak.c > diff --git a/platform/linux-generic/include/api/odp_debug.h > b/platform/linux-generic/include/api/odp_debug.h > index e853be4..4b51038 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" { > @@ -81,61 +82,75 @@ typedef enum odp_log_level { > } odp_log_level_e; > > /** > - * ODP default LOG macro. > + * ODP log function > + * > + * Instead of direct prints to stdout/stderr all logging in ODP > implementation > + * should be done via this function or its wrappers. > + * ODP platform MUST provide a default *weak* implementation of this > function. > MUST -> must > + * Application MAY override the function if needed by providing a strong > MAY -> may > + * function. > + * > + * @param level Log level > doxygen in or out > + * @param fmt printf-style message format > doxygen in or out > + */ > need @return description or if possible @retval if there are specific cases > +extern int odp_override_log(odp_log_level_e level, const char *fmt, ...); > level does not appear to be the best name, these are output streams with different characteristics rather than a single stream with different levels > + > +/** > + * ODP LOG macro. > */ > #define ODP_LOG(level, fmt, ...) \ > 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__, \ > + odp_override_log(level, "%s:%d:%s():" fmt, > __FILE__, \ > __LINE__, __func__, ##__VA_ARGS__); \ > break; \ > case ODP_LOG_PRINT: \ > - fprintf(stdout, " " fmt, ##__VA_ARGS__); \ > + odp_override_log(level, " " fmt, ##__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) > > /** > - * Printing macro, which prints output when the application > - * calls one of the ODP APIs specifically for dumping internal data. > + * Log print message when the application calls one of the ODP APIs > + * specifically for dumping internal data. > */ > #define ODP_PRINT(fmt, ...) \ > ODP_LOG(ODP_LOG_PRINT, fmt, ##__VA_ARGS__) > > /** > - * Debug printing macro, which prints output when DEBUG flag is set. > + * Log debug message if DEBUG flag is set. > The flag is ODP_DEBUG_PRINT > */ > #define ODP_DBG(fmt, ...) \ > ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__) > > /** > - * Print output to stderr (file, line and function). > + * Log error message. > We should say that this is not maskable > */ > #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_weak.c > b/platform/linux-generic/odp_weak.c > new file mode 100644 > index 0000000..fccbc3f > --- /dev/null > +++ b/platform/linux-generic/odp_weak.c > @@ -0,0 +1,23 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#include <odp_internal.h> > +#include <odp_debug.h> > +#include <odp_debug_internal.h> > +#include <odp_hints.h> > + > +ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED, > + const char *fmt, ...) > Why is this ODP_UNUSED, is it needed ? To use it a reimplementation of this function by an application would effectively have to have another switch statement. Generating that need for two switches implies we should be exporting overrides for each log type and not prematurely factorising our code. If we do that we just remove our switch statement and put the implementation under the weak function that each macro directly calls. I think Zoltan is bringing this up on the other patch as well > +{ > + va_list args; > + int r; > + > + va_start(args, fmt); > + r = vfprintf(stderr, fmt, args); > + va_end(args); > + > + return r; > +} > -- > 1.7.9.5 > > > _______________________________________________ > 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
