From 06de5b3803c5194d53b4bde10a41e3a292ad7d44 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 12 Aug 2016 17:59:45 +0100 Subject: [PATCH 01/10] _dbus_logv: configurably log to syslog and/or stderr This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was not called. Previously, _dbus_logv() would always log to syslog; additionally, it would log to stderr, unless the process is dbus-daemon and it was started by systemd. Now, it will log to stderr only, unless _dbus_init_system_log() was called first. This is the desired behaviour because when we hook up _dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't want typical users of libdbus to start logging their check failures to syslog - we only want the dbus-daemon to do that. In practice this is not usually a behaviour change, because there was only one situation in which we called _dbus_logv() without first calling _dbus_init_system_log(), namely an error while parsing configuration files. Initialize the system log "just in time" in that situation to preserve existing behaviour. Signed-off-by: Simon McVittie --- bus/bus.c | 25 ++++++++++++-- bus/config-parser.c | 10 +++--- dbus/dbus-sysdeps-unix.c | 89 ++++++++++++++++++++++++++---------------------- dbus/dbus-sysdeps-win.c | 42 ++++++++++++++++++----- dbus/dbus-sysdeps.c | 2 +- dbus/dbus-sysdeps.h | 9 +++-- test/internals/syslog.c | 9 +++-- 7 files changed, 125 insertions(+), 61 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index 103447c..692f22f 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -47,6 +47,10 @@ #include #endif +#ifdef HAVE_SYSTEMD +#include +#endif + struct BusContext { int refcount; @@ -288,7 +292,25 @@ process_config_first_time_only (BusContext *context, auth_mechanisms = NULL; pidfile = NULL; - _dbus_init_system_log ("dbus-daemon", TRUE); + context->syslog = bus_config_parser_get_syslog (parser); + + if (context->syslog) + { +#ifdef HAVE_SYSTEMD + /* If running under systemd, we don't want to log to both stderr and + * syslog, because our stderr is probably connected to journald too, + * so we'd duplicate all our messages. */ + if (sd_booted () > 0) + _dbus_init_system_log ("dbus-daemon", DBUS_LOG_FLAGS_SYSTEM_LOG); + else +#endif + _dbus_init_system_log ("dbus-daemon", + DBUS_LOG_FLAGS_SYSTEM_LOG | DBUS_LOG_FLAGS_STDERR); + } + else + { + _dbus_init_system_log ("dbus-daemon", DBUS_LOG_FLAGS_STDERR); + } if (flags & BUS_CONTEXT_FLAG_SYSTEMD_ACTIVATION) context->systemd_activation = TRUE; @@ -470,7 +492,6 @@ process_config_first_time_only (BusContext *context, } context->fork = bus_config_parser_get_fork (parser); - context->syslog = bus_config_parser_get_syslog (parser); context->keep_umask = bus_config_parser_get_keep_umask (parser); context->allow_anonymous = bus_config_parser_get_allow_anonymous (parser); diff --git a/bus/config-parser.c b/bus/config-parser.c index 15d4228..2bedc70 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -2310,11 +2310,13 @@ include_dir (BusConfigParser *parser, { if (dbus_error_is_set (error)) { - /* We log to syslog unconditionally here, because this is + /* We use both syslog and stderr here, because this is * the configuration parser, so we don't yet know whether - * this bus is going to want to write to syslog! (There's - * also some layer inversion going on, if we want to use - * the bus context.) */ + * this bus is going to want to write to syslog! Err on + * the side of making sure the message gets to the sysadmin + * somehow. */ + _dbus_init_system_log ("dbus-daemon", + DBUS_LOG_FLAGS_STDERR | DBUS_LOG_FLAGS_SYSTEM_LOG); _dbus_log (DBUS_SYSTEM_LOG_INFO, "Encountered error '%s' while parsing '%s'", error->message, diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 1f964b5..a207230 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -4524,6 +4524,9 @@ _dbus_restore_socket_errno (int saved_errno) } static const char *syslog_tag = "dbus"; +#ifdef HAVE_SYSLOG_H +static DBusLogFlags log_flags = DBUS_LOG_FLAGS_STDERR; +#endif /** * Initialize the system log. @@ -4532,30 +4535,33 @@ static const char *syslog_tag = "dbus"; * the process or until _dbus_init_system_log() is called again. In practice * it will normally be a constant. * + * On platforms that do not support a system log, the + * #DBUS_LOG_FLAGS_SYSTEM_LOG flag is treated as equivalent to + * #DBUS_LOG_FLAGS_STDERR. + * * @param tag the name of the executable (syslog tag) - * @param is_daemon #TRUE if this is the dbus-daemon + * @param mode whether to log to stderr, the system log or both */ void -_dbus_init_system_log (const char *tag, - dbus_bool_t is_daemon) +_dbus_init_system_log (const char *tag, + DBusLogFlags flags) { -#ifdef HAVE_SYSLOG_H - int logopts = LOG_PID; - -#if HAVE_DECL_LOG_PERROR -#ifdef HAVE_SYSTEMD - if (!is_daemon || sd_booted () <= 0) -#endif - logopts |= LOG_PERROR; -#endif + /* We never want to turn off logging completely */ + _dbus_assert ( + (flags & (DBUS_LOG_FLAGS_STDERR | DBUS_LOG_FLAGS_SYSTEM_LOG)) != 0); syslog_tag = tag; - openlog (tag, logopts, LOG_DAEMON); + +#ifdef HAVE_SYSLOG_H + log_flags = flags; + + if (log_flags & DBUS_LOG_FLAGS_SYSTEM_LOG) + openlog (tag, LOG_PID, LOG_DAEMON); #endif } /** - * Log a message to the system log file (e.g. syslog on Unix). + * Log a message to the system log file (e.g. syslog on Unix) and/or stderr. * * @param severity a severity value * @param msg a printf-style format string @@ -4571,40 +4577,43 @@ _dbus_logv (DBusSystemLogSeverity severity, { va_list tmp; #ifdef HAVE_SYSLOG_H - int flags; - switch (severity) - { - case DBUS_SYSTEM_LOG_INFO: - flags = LOG_DAEMON | LOG_NOTICE; - break; - case DBUS_SYSTEM_LOG_WARNING: - flags = LOG_DAEMON | LOG_WARNING; - break; - case DBUS_SYSTEM_LOG_SECURITY: - flags = LOG_AUTH | LOG_NOTICE; - break; - case DBUS_SYSTEM_LOG_FATAL: - flags = LOG_DAEMON|LOG_CRIT; - break; - default: - return; - } - - DBUS_VA_COPY (tmp, args); - vsyslog (flags, msg, tmp); - va_end (tmp); -#endif + if (log_flags & DBUS_LOG_FLAGS_SYSTEM_LOG) + { + int flags; + switch (severity) + { + case DBUS_SYSTEM_LOG_INFO: + flags = LOG_DAEMON | LOG_NOTICE; + break; + case DBUS_SYSTEM_LOG_WARNING: + flags = LOG_DAEMON | LOG_WARNING; + break; + case DBUS_SYSTEM_LOG_SECURITY: + flags = LOG_AUTH | LOG_NOTICE; + break; + case DBUS_SYSTEM_LOG_FATAL: + flags = LOG_DAEMON|LOG_CRIT; + break; + default: + return; + } -#if !defined(HAVE_SYSLOG_H) || !HAVE_DECL_LOG_PERROR + DBUS_VA_COPY (tmp, args); + vsyslog (flags, msg, tmp); + va_end (tmp); + } + + /* If we don't have syslog.h, we always behave as though stderr was in + * the flags */ + if (log_flags & DBUS_LOG_FLAGS_STDERR) +#endif { - /* vsyslog() won't write to stderr, so we'd better do it */ DBUS_VA_COPY (tmp, args); fprintf (stderr, "%s[" DBUS_PID_FORMAT "]: ", syslog_tag, _dbus_getpid ()); vfprintf (stderr, msg, tmp); fputc ('\n', stderr); va_end (tmp); } -#endif if (severity == DBUS_SYSTEM_LOG_FATAL) exit (1); diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index 32b49b8..a3f574f 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -3634,6 +3634,9 @@ _dbus_restore_socket_errno (int saved_errno) _dbus_win_set_errno (saved_errno); } +static const char *log_tag = "dbus"; +static DBusLogFlags log_flags = DBUS_LOG_FLAGS_STDERR; + /** * Initialize the system log. * @@ -3642,13 +3645,18 @@ _dbus_restore_socket_errno (int saved_errno) * it will normally be a constant. * * @param tag the name of the executable (syslog tag) - * @param is_daemon #TRUE if this is the dbus-daemon + * @param mode whether to log to stderr, the system log or both */ void -_dbus_init_system_log (const char *tag, - dbus_bool_t is_daemon) +_dbus_init_system_log (const char *tag, + DBusLogFlags flags) { - /* OutputDebugStringA doesn't need any special initialization, do nothing */ + /* We never want to turn off logging completely */ + _dbus_assert ( + (flags & (DBUS_LOG_FLAGS_STDERR | DBUS_LOG_FLAGS_SYSTEM_LOG)) != 0); + + log_tag = tag; + log_flags = flags; } /** @@ -3667,8 +3675,7 @@ _dbus_logv (DBusSystemLogSeverity severity, va_list args) { char *s = ""; - char buf[1024]; - char format[1024]; + va_list tmp; switch(severity) { @@ -3678,9 +3685,26 @@ _dbus_logv (DBusSystemLogSeverity severity, case DBUS_SYSTEM_LOG_FATAL: s = "fatal"; break; } - snprintf(format, sizeof(format), "%s%s", s ,msg); - vsnprintf(buf, sizeof(buf), format, args); - OutputDebugStringA(buf); + if (log_flags & DBUS_LOG_FLAGS_SYSTEM_LOG) + { + char buf[1024]; + char format[1024]; + + DBUS_VA_COPY (tmp, args); + snprintf (format, sizeof (format), "%s: %s", s, msg); + vsnprintf(buf, sizeof(buf), format, tmp); + OutputDebugStringA(buf); + va_end (tmp); + } + + if (log_flags & DBUS_LOG_FLAGS_STDERR) + { + DBUS_VA_COPY (tmp, args); + fprintf (stderr, "%s[%lu]: %s: ", log_tag, _dbus_pid_for_log (), s); + vfprintf (stderr, msg, tmp); + fprintf (stderr, "\n"); + va_end (tmp); + } if (severity == DBUS_SYSTEM_LOG_FATAL) exit (1); diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index d004ff0..c58377b 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -752,7 +752,7 @@ _dbus_strerror_from_errno (void) } /** - * Log a message to the system log file (e.g. syslog on Unix). + * Log a message to the system log file (e.g. syslog on Unix) and/or stderr. * * @param severity a severity value * @param msg a printf-style format string diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index beb2f1e..a2dac6c 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -555,9 +555,14 @@ void _dbus_set_signal_handler (int sig, dbus_bool_t _dbus_user_at_console (const char *username, DBusError *error); +typedef enum { + DBUS_LOG_FLAGS_STDERR = (1 << 0), + DBUS_LOG_FLAGS_SYSTEM_LOG = (1 << 1) +} DBusLogFlags; + DBUS_PRIVATE_EXPORT -void _dbus_init_system_log (const char *tag, - dbus_bool_t is_daemon); +void _dbus_init_system_log (const char *tag, + DBusLogFlags flags); typedef enum { DBUS_SYSTEM_LOG_INFO, diff --git a/test/internals/syslog.c b/test/internals/syslog.c index c2467be..789ee63 100644 --- a/test/internals/syslog.c +++ b/test/internals/syslog.c @@ -56,7 +56,8 @@ test_syslog (Fixture *f, #ifndef G_OS_WIN32 if (g_test_trap_fork (0, 0)) { - _dbus_init_system_log ("test-syslog", FALSE); + _dbus_init_system_log ("test-syslog", + DBUS_LOG_FLAGS_SYSTEM_LOG | DBUS_LOG_FLAGS_STDERR); _dbus_log (DBUS_SYSTEM_LOG_FATAL, MESSAGE "%d", 23); /* should not be reached: exit 0 so the assertion in the main process * will fail */ @@ -68,7 +69,8 @@ test_syslog (Fixture *f, if (g_test_trap_fork (0, 0)) { - _dbus_init_system_log ("test-syslog", FALSE); + _dbus_init_system_log ("test-syslog", + DBUS_LOG_FLAGS_SYSTEM_LOG | DBUS_LOG_FLAGS_STDERR); _dbus_log (DBUS_SYSTEM_LOG_INFO, MESSAGE "%d", 42); _dbus_log (DBUS_SYSTEM_LOG_WARNING, MESSAGE "%d", 45); _dbus_log (DBUS_SYSTEM_LOG_SECURITY, MESSAGE "%d", 666); @@ -79,7 +81,8 @@ test_syslog (Fixture *f, g_test_trap_assert_stderr ("*" MESSAGE "42\n*" MESSAGE "45\n*" MESSAGE "666\n*"); #endif /* manual test (this is the best we can do on Windows) */ - _dbus_init_system_log ("test-syslog", FALSE); + _dbus_init_system_log ("test-syslog", + DBUS_LOG_FLAGS_SYSTEM_LOG | DBUS_LOG_FLAGS_STDERR); _dbus_log (DBUS_SYSTEM_LOG_INFO, MESSAGE "%d", 42); _dbus_log (DBUS_SYSTEM_LOG_WARNING, MESSAGE "%d", 45); _dbus_log (DBUS_SYSTEM_LOG_SECURITY, MESSAGE "%d", 666); -- 2.8.1