Merge "Make klog_fd thread-safe and make klog_init a no-op."

11 files changed, 33 insertions, 49 deletions

diff --git a/fs_mgr/fs_mgr_main.c b/fs_mgr/fs_mgr_main.c
index e5a00d595..b3f131ea6 100644
--- a/fs_mgr/fs_mgr_main.c
+++ b/fs_mgr/fs_mgr_main.c
@@ -85,7 +85,6 @@ int main(int argc, char *argv[])
     char *fstab_file=NULL;
     struct fstab *fstab=NULL;
 
-    klog_init();
     klog_set_level(6);
 
     parse_options(argc, argv, &a_flag, &u_flag, &n_flag, &n_name, &n_blk_dev);
@@ -111,4 +110,3 @@ int main(int argc, char *argv[])
     /* Should not get here */
     exit(1);
 }
-
diff --git a/include/cutils/klog.h b/include/cutils/klog.h
index 295d62be3..2078fa218 100644
--- a/include/cutils/klog.h
+++ b/include/cutils/klog.h
@@ -26,7 +26,6 @@ __BEGIN_DECLS
 void klog_init(void);
 int  klog_get_level(void);
 void klog_set_level(int level);
-/* TODO: void klog_close(void); - and make klog_fd users thread safe. */
 
 void klog_write(int level, const char *fmt, ...)
     __attribute__ ((format(printf, 2, 3)));
diff --git a/init/init.cpp b/init/init.cpp
index 1495c6820..fe2aee4eb 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -477,7 +477,6 @@ int main(int argc, char** argv) {
     // kmsg and null, otherwise we won't be able to remount / read-only
     // later on. Now that tmpfs is mounted on /dev, we can actually talk
     // to the outside world.
-    open_devnull_stdio();
     InitKernelLogging(argv);
 
     LOG(INFO) << "init " << (is_first_stage ? "first stage" : "second stage") << " started!";
diff --git a/init/log.cpp b/init/log.cpp
index ee75ffc86..3934ca02f 100644
--- a/init/log.cpp
+++ b/init/log.cpp
@@ -16,8 +16,11 @@
 
 #include "log.h"
 
+#include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/uio.h>
 
 #include <selinux/selinux.h>
@@ -51,9 +54,20 @@ static void KernelLogger(android::base::LogId, android::base::LogSeverity severi
 }
 
 void InitKernelLogging(char* argv[]) {
-    android::base::InitLogging(argv, &KernelLogger);
+    // Make stdin/stdout/stderr all point to /dev/null.
+    int fd = open("/sys/fs/selinux/null", O_RDWR);
+    if (fd == -1) {
+        int saved_errno = errno;
+        android::base::InitLogging(argv, &KernelLogger);
+        errno = saved_errno;
+        PLOG(FATAL) << "Couldn't open /sys/fs/selinux/null";
+    }
+    dup2(fd, 0);
+    dup2(fd, 1);
+    dup2(fd, 2);
+    if (fd > 2) close(fd);
 
-    klog_init();
+    android::base::InitLogging(argv, &KernelLogger);
     klog_set_level(KLOG_NOTICE_LEVEL);
 }
 
diff --git a/init/readme.txt b/init/readme.txt
index 27c5e673c..7260775f6 100644
--- a/init/readme.txt
+++ b/init/readme.txt
@@ -515,5 +515,5 @@ Alternatively, use the emulator:
 
   emulator -partition-size 1024 -verbose -show-kernel -no-window
 
-You might want to call klog_set_level(6) after the klog_init() call
-so you see the kernel logging in dmesg (or the emulator output).
+You might want to change the klog_set_level call so you see all the kernel
+logging in dmesg (or the emulator output).
diff --git a/init/ueventd.cpp b/init/ueventd.cpp
index 91f8b1ad2..e7794ec1d 100644
--- a/init/ueventd.cpp
+++ b/init/ueventd.cpp
@@ -52,7 +52,6 @@ int ueventd_main(int argc, char **argv)
      */
     signal(SIGCHLD, SIG_IGN);
 
-    open_devnull_stdio();
     InitKernelLogging(argv);
 
     LOG(INFO) << "ueventd started!";
diff --git a/init/util.cpp b/init/util.cpp
index 6315a7acf..6c1923f8b 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -316,30 +316,6 @@ int wait_for_file(const char *filename, int timeout)
     return ret;
 }
 
-void open_devnull_stdio(void)
-{
-    int fd = open("/sys/fs/selinux/null", O_RDWR);
-    if (fd == -1) {
-        /* Fail silently.
-         * stdout/stderr isn't available, and because
-         * klog_init() is called after open_devnull_stdio(), we can't
-         * log to dmesg. Reordering klog_init() to be called before
-         * open_devnull_stdio() isn't an option either, as then klog_fd
-         * will be assigned 0 or 1, which will end up getting clobbered
-         * by the code below. There's nowhere good to log.
-         */
-
-        exit(1);
-    }
-
-    dup2(fd, 0);
-    dup2(fd, 1);
-    dup2(fd, 2);
-    if (fd > 2) {
-        close(fd);
-    }
-}
-
 void import_kernel_cmdline(bool in_qemu,
                            std::function<void(const std::string&, const std::string&, bool)> fn) {
     std::string cmdline;
diff --git a/init/util.h b/init/util.h
index 45207eb52..9d522cc8e 100644
--- a/init/util.h
+++ b/init/util.h
@@ -54,7 +54,6 @@ void sanitize(char *p);
 void make_link_init(const char *oldpath, const char *newpath);
 void remove_link(const char *oldpath, const char *newpath);
 int wait_for_file(const char *filename, int timeout);
-void open_devnull_stdio(void);
 void import_kernel_cmdline(bool in_qemu,
                            std::function<void(const std::string&, const std::string&, bool)>);
 int make_dir(const char *path, mode_t mode);
diff --git a/init/watchdogd.cpp b/init/watchdogd.cpp
index 3a3d81028..b196147d4 100644
--- a/init/watchdogd.cpp
+++ b/init/watchdogd.cpp
@@ -28,7 +28,6 @@
 #define DEV_NAME "/dev/watchdog"
 
 int watchdogd_main(int argc, char **argv) {
-    open_devnull_stdio();
     InitKernelLogging(argv);
 
     int interval = 10;
diff --git a/libcutils/Android.mk b/libcutils/Android.mk
index e29c9ad6e..c830182f9 100644
--- a/libcutils/Android.mk
+++ b/libcutils/Android.mk
@@ -95,7 +95,7 @@ LOCAL_SRC_FILES := $(libcutils_common_sources) \
         android_reboot.c \
         ashmem-dev.c \
         debugger.c \
-        klog.c \
+        klog.cpp \
         partition_utils.c \
         properties.c \
         qtaguid.c \
diff --git a/libcutils/klog.c b/libcutils/klog.cpp
index 7402903ea..11ebf88fb 100644
--- a/libcutils/klog.c
+++ b/libcutils/klog.cpp
@@ -26,7 +26,6 @@
 
 #include <cutils/klog.h>
 
-static int klog_fd = -1;
 static int klog_level = KLOG_DEFAULT_LEVEL;
 
 int klog_get_level(void) {
@@ -38,31 +37,33 @@ void klog_set_level(int level) {
 }
 
 void klog_init(void) {
-    if (klog_fd >= 0) return; /* Already initialized */
-
-    klog_fd = open("/dev/kmsg", O_WRONLY | O_CLOEXEC);
-    if (klog_fd >= 0) {
-        return;
-    }
+}
 
-    static const char* name = "/dev/__kmsg__";
-    if (mknod(name, S_IFCHR | 0600, (1 << 8) | 11) == 0) {
-        klog_fd = open(name, O_WRONLY | O_CLOEXEC);
-        unlink(name);
+static int __open_klog(void) {
+    int fd = open("/dev/kmsg", O_WRONLY | O_CLOEXEC);
+    if (fd == -1) {
+        static const char* name = "/dev/__kmsg__";
+        if (mknod(name, S_IFCHR | 0600, (1 << 8) | 11) == 0) {
+            fd = open(name, O_WRONLY | O_CLOEXEC);
+            unlink(name);
+        }
     }
+    return fd;
 }
 
 #define LOG_BUF_MAX 512
 
 void klog_writev(int level, const struct iovec* iov, int iov_count) {
     if (level > klog_level) return;
-    if (klog_fd < 0) klog_init();
-    if (klog_fd < 0) return;
+
+    static int klog_fd = __open_klog();
+    if (klog_fd == -1) return;
     TEMP_FAILURE_RETRY(writev(klog_fd, iov, iov_count));
 }
 
 void klog_write(int level, const char* fmt, ...) {
     if (level > klog_level) return;
+
     char buf[LOG_BUF_MAX];
     va_list ap;
     va_start(ap, fmt);