From 168394fb2f40024795600fb0fe2f20fb4e5cf5db Mon Sep 17 00:00:00 2001 From: Rodrigo Kumpera Date: Wed, 31 Aug 2016 17:37:35 -0700 Subject: [PATCH] [logging] Ensure that glib logging goes through the new logging mechanics and that the domain prefix is preserved. Call g_log_set_default_handler in mono_trace_set_log_handler and mono_trace_set_log_handler_internal to ensure that g_ logging is properly routed. Additionally, change the internal logging callback to take the log message as a single string to simplify things. --- mono/utils/mono-log-common.c | 27 +++++++-------- mono/utils/mono-log-posix.c | 8 ++--- mono/utils/mono-log-windows.c | 26 +++++---------- mono/utils/mono-logger-internals.h | 6 ++-- mono/utils/mono-logger.c | 53 ++++++++++++++++++++---------- 5 files changed, 63 insertions(+), 57 deletions(-) diff --git a/mono/utils/mono-log-common.c b/mono/utils/mono-log-common.c index fa11aabcb7b..cf73d9acbb8 100644 --- a/mono/utils/mono-log-common.c +++ b/mono/utils/mono-log-common.c @@ -25,7 +25,7 @@ #else #include #endif -#include "mono-logger.h" +#include "mono-logger-internals.h" static FILE *logFile = NULL; static void *logUserData = NULL; @@ -98,19 +98,17 @@ mono_log_open_logfile(const char *path, void *userData) * @vargs - Variable argument list */ void -mono_log_write_logfile(const char *domain, GLogLevelFlags level, mono_bool hdr, const char *format, va_list args) +mono_log_write_logfile (const char *log_domain, GLogLevelFlags level, mono_bool hdr, const char *message) { time_t t; - char logTime[80], - logMessage[512]; - pid_t pid; - int iLog = 0; - size_t nLog; if (logFile == NULL) logFile = stdout; if (hdr) { + pid_t pid; + char logTime [80]; + #ifndef HOST_WIN32 struct tm tod; time(&t); @@ -124,15 +122,14 @@ mono_log_write_logfile(const char *domain, GLogLevelFlags level, mono_bool hdr, pid = _getpid(); strftime(logTime, sizeof(logTime), "%F %T", tod); #endif - iLog = sprintf(logMessage, "%s level[%c] mono[%d]: ", - logTime,mapLogFileLevel(level),pid); + fprintf (logFile, "%s level[%c] mono[%d]: %s\n", logTime, mapLogFileLevel (level), pid, message); + } else { + fprintf (logFile, "%s%s%s\n", + log_domain != NULL ? log_domain : "", + log_domain != NULL ? ": " : "", + message); } - nLog = sizeof(logMessage) - iLog - 2; - vsnprintf(logMessage+iLog, nLog, format, args); - iLog = strlen(logMessage); - logMessage[iLog++] = '\n'; - logMessage[iLog++] = '\0'; - fputs(logMessage, logFile); + fflush(logFile); if (level == G_LOG_FLAG_FATAL) diff --git a/mono/utils/mono-log-posix.c b/mono/utils/mono-log-posix.c index 1ce111c1762..388fd26202f 100644 --- a/mono/utils/mono-log-posix.c +++ b/mono/utils/mono-log-posix.c @@ -25,7 +25,7 @@ #include #include #include -#include "mono-logger.h" +#include "mono-logger-internals.h" static void *logUserData = NULL; @@ -70,7 +70,7 @@ mono_log_open_syslog(const char *ident, void *userData) } /** - * mono_log_write_logfile + * mono_log_write_syslog * * Write data to the log file. * @@ -80,9 +80,9 @@ mono_log_open_syslog(const char *ident, void *userData) * @vargs - Variable argument list */ void -mono_log_write_syslog(const char *domain, GLogLevelFlags level, mono_bool hdr, const char *format, va_list args) +mono_log_write_syslog(const char *domain, GLogLevelFlags level, mono_bool hdr, const char *message) { - vsyslog(mapSyslogLevel(level), format, args); + syslog (mapSyslogLevel(level), "%s", message); if (level == G_LOG_FLAG_FATAL) abort(); diff --git a/mono/utils/mono-log-windows.c b/mono/utils/mono-log-windows.c index 1746128a938..d0ee0199437 100644 --- a/mono/utils/mono-log-windows.c +++ b/mono/utils/mono-log-windows.c @@ -23,7 +23,7 @@ #include #include #include -#include "mono-logger.h" +#include "mono-logger-internals.h" static FILE *logFile = NULL; static void *logUserData = NULL; @@ -85,31 +85,23 @@ mono_log_open_syslog(const char *ident, void *userData) * @vargs - Variable argument list */ void -mono_log_write_syslog(const char *domain, GLogLevelFlags level, mono_bool hdr, const char *format, va_list args) +mono_log_write_syslog(const char *domain, GLogLevelFlags level, mono_bool hdr, const char *message) { time_t t; - struct tm *tod; - char logTime[80], - logMessage[512]; pid_t pid; - int iLog = 0; - size_t nLog; + char logTime [80]; if (logFile == NULL) - mono_log_open_syslog(NULL, NULL); + logFile = stdout; + struct tm *tod; time(&t); tod = localtime(&t); pid = _getpid(); - strftime(logTime, sizeof(logTime), "%Y-%m-%d %H:%M:%S", tod); - iLog = sprintf(logMessage, "%s level[%c] mono[%d]: ", - logTime,mapLogFileLevel(level),pid); - nLog = sizeof(logMessage) - iLog - 2; - vsnprintf(logMessage+iLog, nLog, format, args); - iLog = strlen(logMessage); - logMessage[iLog++] = '\n'; - logMessage[iLog++] = 0; - fputs(logMessage, logFile); + strftime(logTime, sizeof(logTime), "%F %T", tod); + + fprintf (logFile, "%s level[%c] mono[%d]: %s\n", logTime, mapLogFileLevel (level), pid, message); + fflush(logFile); if (level == G_LOG_FLAG_FATAL) diff --git a/mono/utils/mono-logger-internals.h b/mono/utils/mono-logger-internals.h index afc3aefdc2b..57ef259677f 100644 --- a/mono/utils/mono-logger-internals.h +++ b/mono/utils/mono-logger-internals.h @@ -151,7 +151,7 @@ mono_trace_message(MonoTraceMask mask, const char *format, ...) /* Internal logging API */ typedef void (*MonoLoggerOpen) (const char *, void *); -typedef void (*MonoLoggerWrite) (const char *, GLogLevelFlags, mono_bool, const char *, va_list); +typedef void (*MonoLoggerWrite) (const char *, GLogLevelFlags, mono_bool, const char *); typedef void (*MonoLoggerClose) (void); typedef struct _MonoLogCallParm_ { @@ -168,11 +168,11 @@ void mono_trace_set_logdest_string (const char *value); void mono_trace_set_logheader_string (const char *value); void mono_log_open_syslog (const char *, void *); -void mono_log_write_syslog (const char *, GLogLevelFlags, mono_bool, const char *, va_list); +void mono_log_write_syslog (const char *, GLogLevelFlags, mono_bool, const char *); void mono_log_close_syslog (void); void mono_log_open_logfile (const char *, void *); -void mono_log_write_logfile (const char *, GLogLevelFlags, mono_bool, const char *, va_list); +void mono_log_write_logfile (const char *, GLogLevelFlags, mono_bool, const char *); void mono_log_close_logfile (void); G_END_DECLS diff --git a/mono/utils/mono-logger.c b/mono/utils/mono-logger.c index f6cdd40b20d..20457c95b67 100644 --- a/mono/utils/mono-logger.c +++ b/mono/utils/mono-logger.c @@ -29,7 +29,7 @@ static MonoLogCallParm logCallback = { typedef struct { MonoLogCallback legacy_callback; gpointer user_data; -} legacyLoggerUserData; +} UserSuppliedLoggerUserData; /** * mono_trace_init: @@ -81,19 +81,19 @@ mono_trace_cleanup (void) void mono_tracev_inner (GLogLevelFlags level, MonoTraceMask mask, const char *format, va_list args) { + char *log_message; if (level_stack == NULL) { mono_trace_init (); if(level > mono_internal_current_level || !(mask & mono_internal_current_mask)) return; } - if (logCallback.opener == NULL) { - logCallback.opener = mono_log_open_logfile; - logCallback.writer = mono_log_write_logfile; - logCallback.closer = mono_log_close_logfile; - logCallback.opener(NULL, NULL); - } - logCallback.writer(mono_log_domain, level, logCallback.header, format, args); + g_assert (logCallback.opener); // mono_trace_init should have provided us with one! + + if (g_vasprintf (&log_message, format, args) < 0) + return; + logCallback.writer (mono_log_domain, level, logCallback.header, log_message); + g_free (log_message); } /** @@ -340,13 +340,19 @@ log_level_get_name (GLogLevelFlags log_level) * logging. We ignore the header request as legacy handlers never had headers. */ static void -callback_adapter(const char *domain, GLogLevelFlags level, mono_bool fatal, const char *fmt, va_list args) +callback_adapter (const char *domain, GLogLevelFlags level, mono_bool fatal, const char *message) { - legacyLoggerUserData *ll = (legacyLoggerUserData *) logCallback.user_data; - const char *msg = g_strdup_vprintf (fmt, args); + UserSuppliedLoggerUserData *ll =logCallback.user_data; - ll->legacy_callback (domain, log_level_get_name(level), msg, fatal, ll->user_data); - g_free ((void *) msg); + ll->legacy_callback (domain, log_level_get_name(level), message, fatal, ll->user_data); +} + +static void +eglib_log_adapter (const gchar *log_domain, GLogLevelFlags log_level, const gchar *message, gpointer user_data) +{ + UserSuppliedLoggerUserData *ll = logCallback.user_data; + + ll->legacy_callback (log_domain, log_level_get_name (log_level), message, log_level & G_LOG_LEVEL_ERROR, ll->user_data); } /** @@ -369,7 +375,7 @@ static void legacy_closer(void) { if (logCallback.user_data != NULL) { - g_free (logCallback.user_data); /* This is a LegacyLoggerUserData struct */ + g_free (logCallback.user_data); /* This is a UserSuppliedLoggerUserData struct */ logCallback.opener = NULL; logCallback.writer = NULL; logCallback.closer = NULL; @@ -386,15 +392,16 @@ legacy_closer(void) * * The log handler replaces the default runtime logger. All logging requests with be routed to it. * If the fatal argument in the callback is true, the callback must abort the current process. The runtime expects that - * execution will not resume after a fatal error. This is for "old-style" or legacy log handers. + * execution will not resume after a fatal error. */ void mono_trace_set_log_handler (MonoLogCallback callback, void *user_data) { - g_assert (callback); + g_assert (callback); + if (logCallback.closer != NULL) logCallback.closer(); - legacyLoggerUserData *ll = g_malloc (sizeof (legacyLoggerUserData)); + UserSuppliedLoggerUserData *ll = g_malloc (sizeof (UserSuppliedLoggerUserData)); ll->legacy_callback = callback; ll->user_data = user_data; logCallback.opener = legacy_opener; @@ -402,6 +409,14 @@ mono_trace_set_log_handler (MonoLogCallback callback, void *user_data) logCallback.closer = legacy_closer; logCallback.user_data = ll; logCallback.dest = NULL; + + g_log_set_default_handler (eglib_log_adapter, user_data); +} + +static void +structured_log_adapter (const gchar *log_domain, GLogLevelFlags log_level, const gchar *message, gpointer user_data) +{ + logCallback.writer (log_domain, log_level, logCallback.header, message); } /** @@ -425,7 +440,9 @@ mono_trace_set_log_handler_internal (MonoLogCallParm *callback, void *user_data) logCallback.closer = callback->closer; logCallback.header = mono_trace_log_header; logCallback.dest = callback->dest; - logCallback.opener(logCallback.dest, user_data); + logCallback.opener (logCallback.dest, user_data); + + g_log_set_default_handler (structured_log_adapter, user_data); } static void -- 2.25.1