From cc2b9e6b20d71c60540f54646c3ec87cf57b0400 Mon Sep 17 00:00:00 2001
From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
Date: Fri, 26 Jan 2018 13:42:53 +0000
Subject: [PATCH] rationalize interface for opening/closing logging

log_open_console() did not switch from stderr to /dev/console, when
"always_reopen_console" was set.  It was necessary to call
log_close_console() first.

By contrast, log_open() did switch between e.g. journald and kmsg according
to the value of "prohibit_ipc".

Let's fix log_open() to respect the values of all the log options, and we
can make log_close_*() private.

Also log_close_console() is changed.  There was some precaution, avoiding
closing the console fd if we are not PID 1.  I think commit 48a601fe made
a little mistake in leaving this in, and it only served to confuse
readers :).

Also I changed systemd-shutdown. Now we have log_set_prohibit_ipc(), let's
use it to clarify that systemd-shutdown is not expected to try and log via
journald (which it is about to kill).  We avoided ever asking it to, but
it's more convenient for the reader if they don't have to think about that.
In that sense, it's similar to using assert() to validate a function's
arguments.
---
 src/basic/log.c     | 28 ++++++++++++++--------------
 src/basic/log.h     |  5 -----
 src/core/main.c     |  1 -
 src/core/manager.c  |  4 +---
 src/core/shutdown.c |  1 -
 5 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/src/basic/log.c b/src/basic/log.c
index 2b15ecb..12ee65a 100644
--- a/src/basic/log.c
+++ b/src/basic/log.c
@@ -83,35 +83,34 @@ static bool prohibit_ipc = false;
  * use here. */
 static char *log_abort_msg = NULL;
 
-void log_close_console(void) {
+static void log_close_console(void) {
 
         if (console_fd < 0)
                 return;
 
-        if (getpid_cached() == 1) {
-                if (console_fd >= 3)
-                        safe_close(console_fd);
+        if (console_fd >= 3)
+                safe_close(console_fd);
 
-                console_fd = -1;
-        }
+        console_fd = -1;
 }
 
 static int log_open_console(void) {
 
-        if (console_fd >= 0)
+        if (!always_reopen_console) {
+                console_fd = STDERR_FILENO;
                 return 0;
+        }
 
-        if (always_reopen_console) {
+        if (console_fd < 3) {
                 console_fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC);
                 if (console_fd < 0)
                         return console_fd;
-        } else
-                console_fd = STDERR_FILENO;
+        }
 
         return 0;
 }
 
-void log_close_kmsg(void) {
+static void log_close_kmsg(void) {
         kmsg_fd = safe_close(kmsg_fd);
 }
 
@@ -127,7 +126,7 @@ static int log_open_kmsg(void) {
         return 0;
 }
 
-void log_close_syslog(void) {
+static void log_close_syslog(void) {
         syslog_fd = safe_close(syslog_fd);
 }
 
@@ -199,7 +198,7 @@ fail:
         return r;
 }
 
-void log_close_journal(void) {
+static void log_close_journal(void) {
         journal_fd = safe_close(journal_fd);
 }
 
@@ -241,7 +240,8 @@ int log_open(void) {
         /* If we don't use the console we close it here, to not get
          * killed by SAK. If we don't use syslog we close it here so
          * that we are not confused by somebody deleting the socket in
-         * the fs. If we don't use /dev/kmsg we still keep it open,
+         * the fs, and to make sure we don't use it if prohibit_ipc is
+         * set. If we don't use /dev/kmsg we still keep it open,
          * because there is no reason to close it. */
 
         if (log_target == LOG_TARGET_NULL) {
diff --git a/src/basic/log.h b/src/basic/log.h
index f2c6cd8..5b5a25b 100644
--- a/src/basic/log.h
+++ b/src/basic/log.h
@@ -94,11 +94,6 @@ int log_open(void);
 void log_close(void);
 void log_forget_fds(void);
 
-void log_close_syslog(void);
-void log_close_journal(void);
-void log_close_kmsg(void);
-void log_close_console(void);
-
 void log_parse_environment_realm(LogRealm realm);
 #define log_parse_environment() \
         log_parse_environment_realm(LOG_REALM)
diff --git a/src/core/main.c b/src/core/main.c
index 245b4c0..56200a8 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -2392,7 +2392,6 @@ int main(int argc, char *argv[]) {
                 /* Running inside a container, as PID 1 */
                 arg_system = true;
                 log_set_target(LOG_TARGET_CONSOLE);
-                log_close_console(); /* force reopen of /dev/console */
                 log_open();
 
                 /* For later on, see above... */
diff --git a/src/core/manager.c b/src/core/manager.c
index 4a78bfb..e837a46 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3543,15 +3543,13 @@ void manager_recheck_journal(Manager *m) {
 
                 /* The journal is fully and entirely up? If so, let's permit logging to it, if that's configured. */
                 log_set_prohibit_ipc(false);
-                log_open();
         } else {
 
                 /* If the journal is down, don't ever log to it, otherwise we might end up deadlocking ourselves as we
                  * might trigger an activation ourselves we can't fulfill */
                 log_set_prohibit_ipc(true);
-                log_close_journal();
-                log_open();
         }
+        log_open();
 }
 
 void manager_set_show_status(Manager *m, ShowStatus mode) {
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index fb882db..167c6a9 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -284,7 +284,6 @@ int main(int argc, char *argv[]) {
         /* journald will die if not gone yet. The log target defaults
          * to console, but may have been changed by command line options. */
 
-        log_close_console(); /* force reopen of /dev/console */
         log_open();
 
         umask(0022);
-- 
2.7.4