[PATCH 1/2] logging: Remove some code duplication
From: Holger Hans Peter Freyther <holger@moiji-mobile.com> Extract the mapping of the subsystem number and the checking for the loglevel to a inline method that is shared between the new and old. --- src/logging.c | 74 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/logging.c b/src/logging.c index c7b1999..7db7101 100644 --- a/src/logging.c +++ b/src/logging.c @@ -310,35 +310,52 @@ err: target->output(target, level, buf); } -/*! \brief vararg version of logging function */ -void osmo_vlogp(int subsys, int level, const char *file, int line, - int cont, const char *format, va_list ap) +static inline int map_subsys(int subsys) { - struct log_target *tar; - if (subsys < 0) subsys = subsys_lib2index(subsys); if (subsys > osmo_log_info->num_cat) subsys = DLGLOBAL; + return subsys; +} + +static inline int check_log_to_target(struct log_target *tar, int subsys, int level) +{ + struct log_category *category; + + category = &tar->categories[subsys]; + + /* subsystem is not supposed to be logged */ + if (!category->enabled) + return 0; + + /* Check the global log level */ + if (tar->loglevel != 0 && level < tar->loglevel) + return 0; + + /* Check the category log level */ + if (tar->loglevel == 0 && category->loglevel != 0 && + level < category->loglevel) + return 0; + + /* TODO: Check the filter/selector too? */ + return 1; +} + +/*! \brief vararg version of logging function */ +void osmo_vlogp(int subsys, int level, const char *file, int line, + int cont, const char *format, va_list ap) +{ + struct log_target *tar; + + subsys = map_subsys(subsys); llist_for_each_entry(tar, &osmo_log_target_list, entry) { - struct log_category *category; int output = 0; va_list bp; - category = &tar->categories[subsys]; - /* subsystem is not supposed to be logged */ - if (!category->enabled) - continue; - - /* Check the global log level */ - if (tar->loglevel != 0 && level < tar->loglevel) - continue; - - /* Check the category log level */ - if (tar->loglevel == 0 && category->loglevel != 0 && - level < category->loglevel) + if (!check_log_to_target(tar, subsys, level)) continue; /* Apply filters here... if that becomes messy we will @@ -871,29 +888,12 @@ int log_check_level(int subsys, unsigned int level) { struct log_target *tar; - if (subsys < 0) - subsys = subsys_lib2index(subsys); - - if (subsys > osmo_log_info->num_cat) - subsys = DLGLOBAL; + subsys = map_subsys(subsys); /* TODO: The following could/should be cached (update on config) */ llist_for_each_entry(tar, &osmo_log_target_list, entry) { - struct log_category *category; - - category = &tar->categories[subsys]; - /* subsystem is not supposed to be logged */ - if (!category->enabled) - continue; - - /* Check the global log level */ - if (tar->loglevel != 0 && level < tar->loglevel) - continue; - - /* Check the category log level */ - if (tar->loglevel == 0 && category->loglevel != 0 && - level < category->loglevel) + if (!check_log_to_target(tar, subsys, level)) continue; /* This might get logged (ignoring filters) */ -- 2.6.3
From: Holger Hans Peter Freyther <holger@moiji-mobile.com> log_check_level and osmo_vlogp share the responsibility for the output of a log message. Once check_level has identified the first target that might have an output osmo_vlogp will continue from this place. In practice we have one (stderr/syslog) or two (stderr+VTY) log outputs so avoiding to re-iterate is not that important but as we have found the right place we can just use it. --- include/osmocom/core/logging.h | 38 +++++++++++++++++++++++--------------- src/logging.c | 21 ++++++++++++--------- tests/logging/logging_test.c | 18 +++++++++++++----- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index e51487b..3f58ddb 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -11,6 +11,8 @@ #include <stdarg.h> #include <osmocom/core/linuxlist.h> +struct log_target; + /*! \brief Maximum number of logging contexts */ #define LOG_MAX_CTX 8 /*! \brief Maximum number of logging filters */ @@ -18,17 +20,21 @@ #define DEBUG + + #ifdef DEBUG #define DEBUGP(ss, fmt, args...) \ do { \ - if (log_check_level(ss, LOGL_DEBUG)) \ - logp(ss, __FILE__, __LINE__, 0, fmt, ## args); \ + struct log_target *osmo_log_tgt; \ + if (log_check_level(ss, LOGL_DEBUG, &osmo_log_tgt)) \ + logp(ss, __FILE__, __LINE__, 0, osmo_log_tgt, fmt, ## args); \ } while(0) #define DEBUGPC(ss, fmt, args...) \ do { \ - if (log_check_level(ss, LOGL_DEBUG)) \ - logp(ss, __FILE__, __LINE__, 1, fmt, ## args); \ + struct log_target *osmo_log_tgt; \ + if (log_check_level(ss, LOGL_DEBUG, &osmo_log_tgt)) \ + logp(ss, __FILE__, __LINE__, 1, osmo_log_tgt, fmt, ## args); \ } while(0) #else @@ -38,9 +44,10 @@ void osmo_vlogp(int subsys, int level, const char *file, int line, - int cont, const char *format, va_list ap); + int cont, struct log_target *initial_target, + const char *format, va_list ap); -void logp(int subsys, const char *file, int line, int cont, const char *format, ...) __attribute__ ((format (printf, 5, 6))); +void logp(int subsys, const char *file, int line, int cont, struct log_target *initial_target, const char *format, ...) __attribute__ ((format (printf, 6, 7))); /*! \brief Log a new message through the Osmocom logging framework * \param[in] ss logging subsystem (e.g. \ref DLGLOBAL) @@ -50,8 +57,9 @@ void logp(int subsys, const char *file, int line, int cont, const char *format, */ #define LOGP(ss, level, fmt, args...) \ do { \ - if (log_check_level(ss, level)) \ - logp2(ss, level, __FILE__, __LINE__, 0, fmt, ##args); \ + struct log_target *osmo_log_tgt; \ + if (log_check_level(ss, level, &osmo_log_tgt)) \ + logp2(ss, level, __FILE__, __LINE__, 0, osmo_log_tgt, fmt, ##args); \ } while(0) /*! \brief Continue a log message through the Osmocom logging framework @@ -62,8 +70,9 @@ void logp(int subsys, const char *file, int line, int cont, const char *format, */ #define LOGPC(ss, level, fmt, args...) \ do { \ - if (log_check_level(ss, level)) \ - logp2(ss, level, __FILE__, __LINE__, 1, fmt, ##args); \ + struct log_target *osmo_log_tgt; \ + if (log_check_level(ss, level, &osmo_log_tgt)) \ + logp2(ss, level, __FILE__, __LINE__, 1, osmo_log_tgt, fmt, ##args); \ } while(0) /*! \brief different log levels */ @@ -107,8 +116,6 @@ struct log_context { void *ctx[LOG_MAX_CTX+1]; }; -struct log_target; - /*! \brief Log filter function */ typedef int log_filter(const struct log_context *ctx, struct log_target *target); @@ -211,10 +218,11 @@ struct log_target { /* use the above macros */ void logp2(int subsys, unsigned int level, const char *file, - int line, int cont, const char *format, ...) - __attribute__ ((format (printf, 6, 7))); + int line, int cont, struct log_target *iniial_target, + const char *format, ...) + __attribute__ ((format (printf, 7, 8))); int log_init(const struct log_info *inf, void *talloc_ctx); -int log_check_level(int subsys, unsigned int level); +int log_check_level(int subsys, unsigned int level, struct log_target **out_tar); /* context management */ void log_reset_context(void); diff --git a/src/logging.c b/src/logging.c index 7db7101..c8a16f8 100644 --- a/src/logging.c +++ b/src/logging.c @@ -345,13 +345,13 @@ static inline int check_log_to_target(struct log_target *tar, int subsys, int le /*! \brief vararg version of logging function */ void osmo_vlogp(int subsys, int level, const char *file, int line, - int cont, const char *format, va_list ap) + int cont, struct log_target *ini_tar, const char *format, va_list ap) { - struct log_target *tar; + struct log_target *tar = ini_tar; subsys = map_subsys(subsys); - llist_for_each_entry(tar, &osmo_log_target_list, entry) { + do { int output = 0; va_list bp; @@ -375,27 +375,28 @@ void osmo_vlogp(int subsys, int level, const char *file, int line, va_copy(bp, ap); _output(tar, subsys, level, file, line, cont, format, bp); va_end(bp); - } + } while (tar->entry.next != &osmo_log_target_list && (tar = llist_entry(tar->entry.next, typeof(*tar), entry))); } /*! \brief logging function used by DEBUGP() macro */ void logp(int subsys, const char *file, int line, int cont, - const char *format, ...) + struct log_target *ini_tar, const char *format, ...) { va_list ap; va_start(ap, format); - osmo_vlogp(subsys, LOGL_DEBUG, file, line, cont, format, ap); + osmo_vlogp(subsys, LOGL_DEBUG, file, line, cont, ini_tar, format, ap); va_end(ap); } /*! \brief logging function used by LOGP() macro */ -void logp2(int subsys, unsigned int level, const char *file, int line, int cont, const char *format, ...) +void logp2(int subsys, unsigned int level, const char *file, int line, int cont, + struct log_target *ini_tar, const char *format, ...) { va_list ap; va_start(ap, format); - osmo_vlogp(subsys, level, file, line, cont, format, ap); + osmo_vlogp(subsys, level, file, line, cont, ini_tar, format, ap); va_end(ap); } @@ -884,7 +885,7 @@ int log_init(const struct log_info *inf, void *ctx) /*! \brief Check whether a log entry will be generated. * \returns != 0 if a log entry might get generated by at least one target */ -int log_check_level(int subsys, unsigned int level) +int log_check_level(int subsys, unsigned int level, struct log_target **out_tar) { struct log_target *tar; @@ -897,10 +898,12 @@ int log_check_level(int subsys, unsigned int level) continue; /* This might get logged (ignoring filters) */ + *out_tar = tar; return 1; } /* We are sure, that this will not be logged. */ + *out_tar = NULL; return 0; } diff --git a/tests/logging/logging_test.c b/tests/logging/logging_test.c index 3c8bac4..023a526 100644 --- a/tests/logging/logging_test.c +++ b/tests/logging/logging_test.c @@ -68,6 +68,7 @@ const struct log_info log_info = { int main(int argc, char **argv) { struct log_target *stderr_target; + struct log_target *select_target; log_init(&log_info, NULL); stderr_target = log_target_create_stderr(); @@ -78,24 +79,31 @@ int main(int argc, char **argv) log_parse_category_mask(stderr_target, "DRLL:DCC"); log_parse_category_mask(stderr_target, "DRLL"); DEBUGP(DCC, "You should not see this\n"); - if (log_check_level(DMM, LOGL_DEBUG) != 0) + if (log_check_level(DMM, LOGL_DEBUG, &select_target) != 0) fprintf(stderr, "log_check_level did not catch this case\n"); + OSMO_ASSERT(select_target == NULL); log_parse_category_mask(stderr_target, "DRLL:DCC"); DEBUGP(DRLL, "You should see this\n"); - OSMO_ASSERT(log_check_level(DRLL, LOGL_DEBUG) != 0); + select_target = NULL; + OSMO_ASSERT(log_check_level(DRLL, LOGL_DEBUG, &select_target) != 0); + OSMO_ASSERT(select_target == stderr_target); DEBUGP(DCC, "You should see this\n"); - OSMO_ASSERT(log_check_level(DCC, LOGL_DEBUG) != 0); + select_target = NULL; + OSMO_ASSERT(log_check_level(DCC, LOGL_DEBUG, &select_target) != 0); + OSMO_ASSERT(select_target == stderr_target); DEBUGP(DMM, "You should not see this\n"); - if (log_check_level(DMM, LOGL_DEBUG) != 0) + select_target = NULL; + if (log_check_level(DMM, LOGL_DEBUG, &select_target) != 0) fprintf(stderr, "log_check_level did not catch this case\n"); + OSMO_ASSERT(select_target == NULL); OSMO_ASSERT(filter_called == 0); log_set_all_filter(stderr_target, 0); DEBUGP(DRLL, "You should not see this and filter is called\n"); OSMO_ASSERT(filter_called == 1); - if (log_check_level(DRLL, LOGL_DEBUG) != 0) + if (log_check_level(DRLL, LOGL_DEBUG, &select_target) != 0) fprintf(stderr, "log_check_level did not catch this case (filter)\n"); -- 2.6.3
Hi, On 21.12.2015 15:39, Holger Hans Peter Freyther wrote:
From: Holger Hans Peter Freyther <holger@moiji-mobile.com>
[...]
/* use the above macros */ void logp2(int subsys, unsigned int level, const char *file, - int line, int cont, const char *format, ...) - __attribute__ ((format (printf, 6, 7))); + int line, int cont, struct log_target *iniial_target, + const char *format, ...) + __attribute__ ((format (printf, 7, 8)));
Note that this will break openbsc/src/ipaccess/ipaccess-proxy.c. Jacob -- - Jacob Erlbeck <jerlbeck@sysmocom.de> http://www.sysmocom.de/ ======================================================================= * sysmocom - systems for mobile communications GmbH * Alt-Moabit 93 * 10559 Berlin, Germany * Sitz / Registered office: Berlin, HRB 134158 B * Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
participants (2)
-
Holger Hans Peter Freyther -
Jacob Erlbeck