internals: Decouple logging an error from exiting unsuccessfully

This lets _dbus_warn() and _dbus_warn_check_failed() fall through
to flushing stderr and calling _dbus_abort(), meaning that failed
checks and warnings can result in a core dump as intended.
By renaming the FATAL severity to ERROR, we ensure that any code
contributions that assumed the old semantics will fail to compile.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568

diff --git a/bus/bus.c b/bus/bus.c
index b013866..27a13ec 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -25,6 +25,7 @@
 #include "bus.h"
 
 #include <stdio.h>
+#include <stdlib.h>
 
 #include "activation.h"
 #include "connection.h"
@@ -963,7 +964,10 @@ bus_context_new (const DBusString *config_file,
 
   if (!bus_selinux_full_init ())
     {
-      bus_context_log (context, DBUS_SYSTEM_LOG_FATAL, "SELinux enabled but D-Bus initialization failed; check system log\n");
+      bus_context_log (context, DBUS_SYSTEM_LOG_ERROR,
+                       "SELinux enabled but D-Bus initialization failed; "
+                       "check system log");
+      exit (1);
     }
 
   if (!bus_apparmor_full_init (error))

diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index bc3454d..267aef9 100644 (file)
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -237,7 +237,7 @@ _dbus_warn (const char *format,
     init_warnings ();
 
   if (fatal_warnings)
-    severity = DBUS_SYSTEM_LOG_FATAL;
+    severity = DBUS_SYSTEM_LOG_ERROR;
 
   va_start (args, format);
   _dbus_logv (severity, format, args);
@@ -269,7 +269,7 @@ _dbus_warn_check_failed(const char *format,
     init_warnings ();
 
   if (fatal_warnings_on_check_failed)
-    severity = DBUS_SYSTEM_LOG_FATAL;
+    severity = DBUS_SYSTEM_LOG_ERROR;
 
   va_start (args, format);
   _dbus_logv (severity, format, args);

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 92217e3..bb05957 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -4614,9 +4614,6 @@ _dbus_init_system_log (const char   *tag,
  * @param severity a severity value
  * @param msg a printf-style format string
  * @param args arguments for the format string
- *
- * If the FATAL severity is given, this function will terminate the program
- * with an error code.
  */
 void
 _dbus_logv (DBusSystemLogSeverity  severity,
@@ -4639,7 +4636,7 @@ _dbus_logv (DBusSystemLogSeverity  severity,
           case DBUS_SYSTEM_LOG_SECURITY:
             flags = LOG_AUTH | LOG_NOTICE;
             break;
-          case DBUS_SYSTEM_LOG_FATAL:
+          case DBUS_SYSTEM_LOG_ERROR:
             flags = LOG_DAEMON|LOG_CRIT;
             break;
           default:
@@ -4662,9 +4659,6 @@ _dbus_logv (DBusSystemLogSeverity  severity,
       fputc ('\n', stderr);
       va_end (tmp);
     }
-
-  if (severity == DBUS_SYSTEM_LOG_FATAL)
-    exit (1);
 }
 
 /* tests in dbus-sysdeps-util.c */

diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index 5a94eaf..74a9501 100644 (file)
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -3702,9 +3702,6 @@ _dbus_init_system_log (const char   *tag,
  * @param severity a severity value
  * @param msg a printf-style format string
  * @param args arguments for the format string
- *
- * If the FATAL severity is given, this function will terminate the program
- * with an error code.
  */
 void
 _dbus_logv (DBusSystemLogSeverity  severity,
@@ -3719,7 +3716,7 @@ _dbus_logv (DBusSystemLogSeverity  severity,
      case DBUS_SYSTEM_LOG_INFO: s = "info"; break;
      case DBUS_SYSTEM_LOG_WARNING: s = "warning"; break;
      case DBUS_SYSTEM_LOG_SECURITY: s = "security"; break;
-     case DBUS_SYSTEM_LOG_FATAL: s = "fatal"; break;
+     case DBUS_SYSTEM_LOG_ERROR: s = "error"; break;
      default: _dbus_assert_not_reached ("invalid log severity");
    }
 
@@ -3743,9 +3740,6 @@ _dbus_logv (DBusSystemLogSeverity  severity,
       fprintf (stderr, "\n");
       va_end (tmp);
     }
-
-  if (severity == DBUS_SYSTEM_LOG_FATAL)
-    exit (1);
 }
 
 /** @} end of sysdeps-win */

diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 281d719..e204fca 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -563,7 +563,7 @@ typedef enum {
   DBUS_SYSTEM_LOG_INFO,
   DBUS_SYSTEM_LOG_WARNING,
   DBUS_SYSTEM_LOG_SECURITY,
-  DBUS_SYSTEM_LOG_FATAL
+  DBUS_SYSTEM_LOG_ERROR
 } DBusSystemLogSeverity;
 
 DBUS_PRIVATE_EXPORT

diff --git a/test/internals/syslog.c b/test/internals/syslog.c
index 4e0b8b8..e69c14a 100644 (file)
--- a/test/internals/syslog.c
+++ b/test/internals/syslog.c
@@ -50,25 +50,6 @@ setup (Fixture *f,
 #define MESSAGE "regression test for _dbus_log(): "
 
 static void
-test_syslog_fatal (Fixture *f,
-    gconstpointer data)
-{
-  if (g_test_subprocess ())
-    {
-      _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 */
-      exit (0);
-    }
-
-  g_test_trap_subprocess (NULL, 0, 0);
-  g_test_trap_assert_failed ();
-  g_test_trap_assert_stderr ("*" MESSAGE "23\n*");
-}
-
-static void
 test_syslog_normal (Fixture *f,
     gconstpointer data)
 {
@@ -79,6 +60,7 @@ test_syslog_normal (Fixture *f,
       _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);
+      _dbus_log (DBUS_SYSTEM_LOG_ERROR, MESSAGE "%d", 23);
 
       _dbus_init_system_log ("test-syslog-stderr", DBUS_LOG_FLAGS_STDERR);
       _dbus_log (DBUS_SYSTEM_LOG_INFO,
@@ -99,6 +81,7 @@ test_syslog_normal (Fixture *f,
   g_test_trap_assert_stderr ("*" MESSAGE "42\n"
                              "*" MESSAGE "45\n"
                              "*" MESSAGE "666\n"
+                             "*" MESSAGE "23\n"
                              "*test-syslog-stderr*" MESSAGE
                                "this should not appear in the syslog\n"
                              "*test-syslog-both*" MESSAGE
@@ -121,8 +104,6 @@ main (int argc,
 {
   test_init (&argc, &argv);
 
-  g_test_add ("/syslog/fatal", Fixture, NULL, setup, test_syslog_fatal,
-              teardown);
   g_test_add ("/syslog/normal", Fixture, NULL, setup, test_syslog_normal,
               teardown);