Hello Martin,
> Thanks for your patchset, looks promising. Sounds good. > I didn't fully understand your requirements for such unique IDs, but > they of course might be helpful for different things. I know charon gives you a great deal of options how to configure logging and I can very well deal with them. But if you look at it from the perspective of someone who does not know the internals of the ike-daemon or does not care if a message comes from cfg facility with debug level 2 or kernel with debug level 1 but only if this message indicates that he/she has to do something or not, you might get my point. Having the hook then allows you for numbering the messages as well as for prepending additional patterns (like inf, err, emerg etc) based on the aforementioned database which can be individually catagorized to ones needs. Also for two messages originating from the identical source -- let's say they contain format specifiers like %Y, %H etc -- might look different to an untrained eye. This might sound a bit far fetched, but believe me, people will not alwas notice the messages are actually identical. So instead of groping in the dark if someone can give you a unique number that can be used to identify the source of it makes resolving bugs a lot more easier (especially if they are caused by misconfigurations). > The question I'm asking is: do you need these IDs across multiple > loggers, i.e. do we have to add this (a little exotic) unique ID > parameter to all loggers? No the IDs are not required across multiple loggers. Hiding the id from all other loggers makes it even less intrusive than I thought (btw thanks for calling it exotic, I think having made it an u_int64_t was a bit strong). > What about extending the logger_t interface by an optional "vlog" method > that takes the raw format string along with a va_list? This would allow > your logger backend to create/map these unique IDs just for itself > without letting other loggers bother with them, and then take > appropriate actions. If you are talking about something like the code that I've (hopefully) attached to this mail, then we're on the same page. It's a much better approach than my original idea. >[...] > > That approach would be a little more flexible, as we are not bound to an > arbitrary 64-bit identifier, but can inspect the raw format string in > the logging backend. Does that sound reasonable? Full ack. Regards, Thomas
>From 8c4c08742a9bf355999ae0890c75c40cbf7cacc7 Mon Sep 17 00:00:00 2001 From: Thomas Egerer <[email protected]> Date: Wed, 3 Apr 2013 22:10:33 +0200 Subject: [PATCH] (POC) Use vlog if available to allow for error unification MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------1.7.1" This is a multi-part message in MIME format. --------------1.7.1 Content-Type: text/plain; charset=UTF-8; format=fixed Content-Transfer-Encoding: 8bit Just a proof of concept, did not bother to compile it... Signed-off-by: Thomas Egerer <[email protected]> --- src/libcharon/bus/bus.c | 11 +++++++++-- src/libcharon/bus/listeners/logger.h | 3 +++ 2 files changed, 12 insertions(+), 2 deletions(-) --------------1.7.1 Content-Type: text/x-patch; name="0001-POC-Use-vlog-if-available-to-allow-for-error-unifica.patch" Content-Transfer-Encoding: 8bit Content-Disposition: inline; filename="0001-POC-Use-vlog-if-available-to-allow-for-error-unifica.patch" diff --git a/src/libcharon/bus/bus.c b/src/libcharon/bus/bus.c index c493291..d397145 100644 --- a/src/libcharon/bus/bus.c +++ b/src/libcharon/bus/bus.c @@ -275,7 +275,8 @@ typedef struct { /** * logger->log() invocation as a invoke_function callback */ -static void log_cb(log_entry_t *entry, log_data_t *data) +static void log_cb(log_entry_t *entry, log_data_t *data, const char *format, + va_list args) { if (entry->levels[data->group] < data->level) { @@ -283,6 +284,11 @@ static void log_cb(log_entry_t *entry, log_data_t *data) } entry->logger->log(entry->logger, data->group, data->level, data->thread, data->ike_sa, data->message); + if (entry->lggger->vlog_cb) + { + entry->logger->vlog(entry->logger, data->group, data->level, + data->thread, data->ike_sa, format, args); + } } METHOD(bus_t, vlog, void, @@ -300,7 +306,8 @@ METHOD(bus_t, vlog, void, data.group = group; data.level = level; vsnprintf(data.message, sizeof(data.message), format, args); - loggers->invoke_function(loggers, (linked_list_invoke_t)log_cb, &data); + loggers->invoke_function(loggers, (linked_list_invoke_t)log_cb, &data, + format, args); } this->log_lock->unlock(this->log_lock); } diff --git a/src/libcharon/bus/listeners/logger.h b/src/libcharon/bus/listeners/logger.h index 0306536..6146ee4 100644 --- a/src/libcharon/bus/listeners/logger.h +++ b/src/libcharon/bus/listeners/logger.h @@ -47,6 +47,9 @@ struct logger_t { void (*log)(logger_t *this, debug_t group, level_t level, int thread, ike_sa_t *ike_sa, char* message); + void (*vlog)(logger_t *this, debug_t group, level_t level, int thread, + ike_sa_t *ike_sa, const char *format, va_list args); + /** * Get the desired log level for a debug group. This is called during * registration. --------------1.7.1--
_______________________________________________ Dev mailing list [email protected] https://lists.strongswan.org/mailman/listinfo/dev
