Hi Harald, On 27/06/11 11:00, Harald Welte wrote: > Hi! > > As more and more code is moving into libraries (like I just did with the > LAPDm code, and like pablo is working on with libosmo-abis), we needed a > solution how to allocate and use the LOGP subsystem constants like DRSL, > DRR, ... from within libraries. > > The existing logging code wasn't really prepared for that. I've now > come um with a hack to extend it while preserving compatibility to > applications: > > * we use negative numbers starting from -1 for library-internal > subsystems > * those numbers get converted to a positive index into the various > arrays at run-time. So -1 ends up one entry higher in the array > than the last application-providede log category/subsystem. > > As part of this change, the array allocations are now dynamic, i.e there > is no maximum limit for the number of log categories that an application > can register with the core. > > Only for libraries (even outside libosmocore), we have compile-time > registration, i.e. the 'struct log_info_cat' and the D* constant need to > be defined inside libosmocore. I think this is an acceptable > compromise. > > Furthermore, if LOGP()/DEBUGP() ever see a subsystem number that it > doesn't know, it will assign it to the new 'DLGLOBAL' (Debug Library > GLOBAL) category, i.e. there cna be no array overflows. > > This ensures that even an external library using a 'newer' D* constant > will not crash or otherwise fail, it will simply log in a slightly > different way.
Since I required something for libosmo-abis I prepared the following patch. The idea is to reduce the logging range to uint8_t: * we reserve logging subsystems space for libraries. * It allows to dinamically register as many logging subsystems as you want, by invoking log_init(...) as many times as needed. This breaks binary backward compatibility.
logging: limit number of signal subsystems to 2^8 From: Pablo Neira Ayuso <[email protected]> This patch limits the number of signal subsystems and types to 256. Basically, we split the signal subsystem/type range in two. The first range (from 0 to 63) can be used by client applications while the second range (from 64 to 255) is reserved for libraries. This patch also allows to register multiple struct log_info definitions from client applications and libraries. Thus, you can invoke: log_init(&you_log_info); as many times as you want, as long as the categories that you register don't clash with the one already registered. This is useful to add support for logging subsystems in libraries (that we require for the upcoming libosmo-abis). --- include/osmocom/core/logging.h | 15 +++++- src/logging.c | 95 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index db02940..dae2e11 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -5,7 +5,14 @@ #include <stdint.h> #include <osmocom/core/linuxlist.h> -#define LOG_MAX_CATEGORY 32 +/* subsystem logging numbers: we split the numberspace for applications and + * libraries: from 0 to 64 for applications, from 64 to 255 for libraries. */ +#define OSMO_LOG_SS_APPS 0 +#define OSMO_LOG_SS_RESERVED 64 +#define OSMO_LOG_SS_ABIS_RESERVED OSMO_LOG_SS_RESERVED +#define OSMO_LOG_SS_MAX 256 + +#define LOG_MAX_CATEGORY OSMO_LOG_SS_MAX #define LOG_MAX_CTX 8 #define LOG_MAX_FILTERS 8 @@ -20,7 +27,7 @@ #endif -void logp(unsigned int subsys, char *file, int line, int cont, const char *format, ...) __attribute__ ((format (printf, 5, 6))); +void logp(uint8_t subsys, char *file, int line, int cont, const char *format, ...) __attribute__ ((format (printf, 5, 6))); /* new logging interface */ #define LOGP(ss, level, fmt, args...) \ @@ -65,7 +72,7 @@ struct log_info { log_filter *filter_fn; /* per-category information */ - const struct log_info_cat *cat; + struct log_info_cat *cat; unsigned int num_cat; }; @@ -110,7 +117,7 @@ struct log_target { }; /* use the above macros */ -void logp2(unsigned int subsys, unsigned int level, char *file, +void logp2(uint8_t subsys, unsigned int level, char *file, int line, int cont, const char *format, ...) __attribute__ ((format (printf, 6, 7))); void log_init(const struct log_info *cat); diff --git a/src/logging.c b/src/logging.c index 8c4d270..d66c7cb 100644 --- a/src/logging.c +++ b/src/logging.c @@ -40,7 +40,7 @@ #include <osmocom/vty/logging.h> /* for LOGGING_STR. */ -const struct log_info *osmo_log_info; +struct log_info *osmo_log_info; static struct log_context log_context; static void *tall_log_ctx = NULL; @@ -84,6 +84,8 @@ int log_parse_category(const char *category) int i; for (i = 0; i < osmo_log_info->num_cat; ++i) { + if (osmo_log_info->cat[i].name == NULL) + continue; if (!strcasecmp(osmo_log_info->cat[i].name+1, category)) return i; } @@ -115,7 +117,8 @@ void log_parse_category_mask(struct log_target* target, const char *_mask) if (colon) length = colon - category_token; - if (strncasecmp(osmo_log_info->cat[i].name, + if (osmo_log_info->cat[i].name != NULL && + strncasecmp(osmo_log_info->cat[i].name, category_token, length) == 0) { int level = 0; @@ -131,7 +134,7 @@ void log_parse_category_mask(struct log_target* target, const char *_mask) free(mask); } -static const char* color(int subsys) +static const char* color(uint8_t subsys) { if (subsys < osmo_log_info->num_cat) return osmo_log_info->cat[subsys].color; @@ -190,7 +193,7 @@ err: } -static void _logp(unsigned int subsys, int level, char *file, int line, +static void _logp(uint8_t subsys, int level, char *file, int line, int cont, const char *format, va_list ap) { struct log_target *tar; @@ -234,7 +237,7 @@ static void _logp(unsigned int subsys, int level, char *file, int line, } } -void logp(unsigned int subsys, char *file, int line, int cont, +void logp(uint8_t subsys, char *file, int line, int cont, const char *format, ...) { va_list ap; @@ -244,7 +247,7 @@ void logp(unsigned int subsys, char *file, int line, int cont, va_end(ap); } -void logp2(unsigned int subsys, unsigned int level, char *file, int line, int cont, const char *format, ...) +void logp2(uint8_t subsys, unsigned int level, char *file, int line, int cont, const char *format, ...) { va_list ap; @@ -441,8 +444,11 @@ const char *log_vty_command_string(const struct log_info *info) int size = strlen("logging level () ()") + 1; char *str; - for (i = 0; i < info->num_cat; i++) + for (i = 0; i < info->num_cat; i++) { + if (info->cat[i].name == NULL) + continue; size += strlen(info->cat[i].name) + 1; + } for (i = 0; i < LOGLEVEL_DEFS; i++) size += strlen(loglevel_strs[i].str) + 1; @@ -458,7 +464,12 @@ const char *log_vty_command_string(const struct log_info *info) OSMO_SNPRINTF_RET(ret, rem, offset, len); for (i = 0; i < info->num_cat; i++) { - int j, name_len = strlen(info->cat[i].name)+1; + int j, name_len; + + if (info->cat[i].name == NULL) + continue; + + name_len = strlen(info->cat[i].name)+1; char name[name_len]; for (j = 0; j < name_len; j++) @@ -555,8 +566,72 @@ err: return str; } -void log_init(const struct log_info *cat) +static void log_info_set(struct log_info_cat *lic1, struct log_info_cat *lic2) { + lic1->name = talloc_strdup(tall_log_ctx, lic2->name); + lic1->color = talloc_strdup(tall_log_ctx, lic2->color); + lic1->description = talloc_strdup(tall_log_ctx, lic2->description); + lic1->loglevel = lic2->loglevel; + lic1->enabled = lic2->enabled; +} + +static void log_info_init(const struct log_info *log_info) +{ + /* keep this easy, allocate maximum number of categories. */ + size_t size = sizeof(struct log_info_cat) * OSMO_LOG_SS_MAX; + int i; + tall_log_ctx = talloc_named_const(NULL, 1, "logging"); - osmo_log_info = cat; + + osmo_log_info = talloc_zero(tall_log_ctx, struct log_info); + if (osmo_log_info == NULL) { + fprintf(stderr, "cannot allocate memory for logging\n"); + return; + } + osmo_log_info->filter_fn = log_info->filter_fn; + osmo_log_info->num_cat = log_info->num_cat; + + osmo_log_info->cat = talloc_zero_size(tall_log_ctx, size); + if (osmo_log_info->cat == NULL) { + fprintf(stderr, "cannot allocate memory for logging\n"); + return; + } + for (i=0; i<log_info->num_cat; i++) { + if (log_info->cat[i].name == NULL) + continue; + log_info_set(&osmo_log_info->cat[i], &log_info->cat[i]); + } +} + +static void log_info_add(const struct log_info *log_info) +{ + int i, limit = osmo_log_info->num_cat; + + if (osmo_log_info->filter_fn == NULL) { + osmo_log_info->filter_fn = log_info->filter_fn; + } else { + fprintf(stderr, "sorry, you cannot override filter " + "function in logging infrastructure\n"); + } + if (log_info->num_cat > osmo_log_info->num_cat) + limit = osmo_log_info->num_cat; + + osmo_log_info->num_cat = limit; + for (i=0; i<log_info->num_cat; i++) { + if (log_info->cat[i].name == NULL) + continue; + if (osmo_log_info->cat[i].name == NULL) + log_info_set(&osmo_log_info->cat[i], &log_info->cat[i]); + } +} + +void log_init(const struct log_info *log_info) +{ + /* special case: we allow that osmo_init_logging(...) takes NULL. */ + if (log_info == NULL) + return; + if (tall_log_ctx == NULL) + log_info_init(log_info); + else + log_info_add(log_info); }
