From 7e56a235fa17681701f4273ce423611740d27792 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 3 Jul 2017 18:58:42 +0100 Subject: [PATCH 19/19] 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 --- bus/bus.c | 6 +++++- dbus/dbus-internals.c | 4 ++-- dbus/dbus-sysdeps-unix.c | 8 +------- dbus/dbus-sysdeps-win.c | 8 +------- dbus/dbus-sysdeps.h | 2 +- test/internals/syslog.c | 23 ++--------------------- 6 files changed, 12 insertions(+), 39 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index b0138664..27a13ec9 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -25,6 +25,7 @@ #include "bus.h" #include +#include #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 bc3454d5..267aef97 100644 --- 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 92217e35..bb059579 100644 --- 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 5a94eaf5..74a95018 100644 --- 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 281d719e..e204fca5 100644 --- 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 4e0b8b83..e69c14a1 100644 --- 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); -- 2.13.2