_dbus_logv: configurably log to syslog and/or stderr
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Fri, 12 Aug 2016 16:59:45 +0000 (17:59 +0100)
committerSimon McVittie <smcv@debian.org>
Fri, 30 Sep 2016 18:36:50 +0000 (19:36 +0100)
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 <smcv@debian.org>
bus/bus.c
bus/config-parser.c
dbus/dbus-sysdeps-unix.c
dbus/dbus-sysdeps-win.c
dbus/dbus-sysdeps.c
dbus/dbus-sysdeps.h
test/internals/syslog.c

index 103447c..692f22f 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
 #include <signal.h>
 #endif
 
+#ifdef HAVE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#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);
 
index 15d4228..2bedc70 100644 (file)
@@ -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,
index 1f964b5..a207230 100644 (file)
@@ -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);
index 32b49b8..a3f574f 100644 (file)
@@ -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);
index d004ff0..c58377b 100644 (file)
@@ -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
index beb2f1e..a2dac6c 100644 (file)
@@ -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,
index c2467be..789ee63 100644 (file)
@@ -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);