Refactor Logging

launcher/src/crash-report.c

@@ -58,7 +58,6 @@ static void exit_handler(__attribute__((unused)) int signal) {
 #define PIPE_WRITE 1
 #define MCPI_LOGS_DIR "/tmp/.minecraft-pi-logs"
 static char log_filename[] = MCPI_LOGS_DIR "/XXXXXX";
-static int log_file_fd = -1;
 void setup_log_file() {
     // Ensure Temporary Directory
     {
@@ -74,16 +73,12 @@ void setup_log_file() {
     }
 
     // Create Temporary File
-    log_file_fd = mkstemp(log_filename);
+    int log_file_fd = mkstemp(log_filename);
     if (log_file_fd == -1) {
         ERR("Unable To Create Log File: %s", strerror(errno));
     }
-
+    close(log_file_fd);
-    // Setup Environment
+    reborn_set_log(log_filename);
-    char *log_file_fd_env = NULL;
-    safe_asprintf(&log_file_fd_env, "%i", log_file_fd);
-    set_and_print_env("MCPI_LOG_FILE_FD", log_file_fd_env);
-    free(log_file_fd_env);
 }
 void setup_crash_report() {
     // Store Output
@@ -176,17 +171,17 @@ void setup_crash_report() {
                                 bytes_available = 0;
                             }
                             // Read
-                            ssize_t bytes_read = read(poll_fds[i].fd, (void *) buf, BUFFER_SIZE);
+                            ssize_t bytes_read = read(poll_fds[i].fd, buf, BUFFER_SIZE);
                             if (bytes_read == -1) {
-                                ERR("Unable To Read Log Data: %s", strerror(errno));
+                                ERR("Unable To Read Input: %s", strerror(errno));
                             }
                             // Write To Child
-                            if (write(input_pipe[PIPE_WRITE], (void *) buf, bytes_read) == -1) {
+                            if (write(input_pipe[PIPE_WRITE], buf, bytes_read) == -1) {
                                 ERR("Unable To Write Input To Child: %s", strerror(errno));
                             }
                         } else {
                             // Data Available From Child's stdout/stderr
-                            ssize_t bytes_read = read(poll_fds[i].fd, (void *) buf, BUFFER_SIZE - 1 /* Account For NULL-Terminator */);
+                            ssize_t bytes_read = read(poll_fds[i].fd, buf, BUFFER_SIZE - 1 /* Account For NULL-Terminator */);
                             if (bytes_read == -1) {
                                 ERR("Unable To Read Log Data: %s", strerror(errno));
                             }
@@ -196,9 +191,11 @@ void setup_crash_report() {
                             fprintf(poll_fds[i].fd == output_pipe[PIPE_READ] ? stdout : stderr, "%s", buf);
 
                             // Write To log
-                            if (write(log_file_fd, (void *) buf, bytes_read) == -1) {
+                            reborn_lock_log();
+                            if (write(reborn_get_log_fd(), buf, bytes_read) == -1) {
                                 ERR("Unable To Write Log Data: %s", strerror(errno));
                             }
+                            reborn_unlock_log();
                         }
                     } else {
                         // File Descriptor No Longer Accessible
@@ -232,18 +229,18 @@ void setup_crash_report() {
             fprintf(stderr, "%s", exit_code_line);
 
             // Write Exit Code Log Line
-            if (write(log_file_fd, (void *) exit_code_line, strlen(exit_code_line)) == -1) {
+            reborn_lock_log();
+            if (write(reborn_get_log_fd(), exit_code_line, strlen(exit_code_line)) == -1) {
                 ERR("Unable To Write Exit Code To Log: %s", strerror(errno));
             }
+            reborn_unlock_log();
 
             // Free Exit Code Log Line
             free(exit_code_line);
         }
 
-        // Close Log File FD
+        // Close Log File
-        if (close(log_file_fd) == -1) {
+        reborn_close_log();
-            ERR("Unable To Close Log File Descriptor: %s", strerror(errno));
-        }
 
         // Show Crash Log
 #ifndef MCPI_HEADLESS_MODE

launcher/src/main.cpp

@@ -135,6 +135,7 @@ int main(int argc, char *argv[]) {
     reborn_debug_tag = "(Launcher) ";
 
     // Debug Logging
+    unsetenv(MCPI_LOG_ENV);
     bind_to_env(MCPI_DEBUG_ENV, options.debug);
 
     // Handle Non-Launch Commands (Copy SDK, Print Feature Flags, Etc)

libreborn/include/libreborn/log.h

@@ -7,15 +7,24 @@
 extern "C" {
 #endif
 
-// Debug
+// Log File
+#define MCPI_LOG_ENV "_MCPI_LOG"
+int reborn_get_log_fd();
+void reborn_lock_log();
+void reborn_unlock_log();
+void reborn_close_log();
+void reborn_set_log(const char *file);
+// Debug Logging
 #define MCPI_DEBUG_ENV "MCPI_DEBUG"
 extern const char *reborn_debug_tag;
 int reborn_get_debug_fd();
+void reborn_lock_debug();
+void reborn_unlock_debug();
 
 // Logging
 #define INFO(format, ...) { fprintf(stderr, "[INFO]: " format "\n", ##__VA_ARGS__); }
 #define WARN(format, ...) { fprintf(stderr, "[WARN]: " format "\n", ##__VA_ARGS__); }
-#define RAW_DEBUG(tag, format, ...) { int debug_fd = reborn_get_debug_fd(); if (debug_fd != -1) { dprintf(debug_fd, "[DEBUG]: %s" format "\n", tag, ##__VA_ARGS__); } }
+#define RAW_DEBUG(tag, format, ...) { reborn_lock_debug(); dprintf(reborn_get_debug_fd(), "[DEBUG]: %s" format "\n", tag, ##__VA_ARGS__); reborn_unlock_debug(); }
 #define DEBUG(format, ...) RAW_DEBUG(reborn_debug_tag, format, ##__VA_ARGS__)
 #define ERR(format, ...) { fprintf(stderr, "[ERR]: (%s:%i): " format "\n", __FILE__, __LINE__, ##__VA_ARGS__); exit(EXIT_FAILURE); }
 #define IMPOSSIBLE() ERR("This Should Never Be Called")

libreborn/src/util/exec.c

@@ -66,13 +66,8 @@ char *run_command(const char *const command[], int *exit_status, size_t *output_
         close(output_pipe[1]);
 
         // Setup stderr
-        if (getenv("MCPI_DEBUG") == NULL) {
+        reborn_lock_debug(); // Lock Released On Process Exit
-            const char *log_file_fd_env = getenv("MCPI_LOG_FILE_FD");
+        dup2(reborn_get_debug_fd(), STDERR_FILENO);
-            if (log_file_fd_env == NULL) {
-                IMPOSSIBLE();
-            }
-            dup2(atoi(log_file_fd_env), STDERR_FILENO);
-        }
 
         // Setup Environment
         setup_exec_environment(0);

libreborn/src/util/log.c

@@ -1,23 +1,73 @@
 #include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/file.h>
 
 #include <libreborn/log.h>
+#include <libreborn/exec.h>
 
 // Debug Tag
 const char *reborn_debug_tag = "";
 
-// Debug FD
+// Log File
-int reborn_get_debug_fd() {
+static int log_fd = -1;
-    if (getenv(MCPI_DEBUG_ENV) != NULL) {
+int reborn_get_log_fd() {
-        return STDERR_FILENO;
+    if (log_fd >= 0) {
-    } else {
+        return log_fd;
-        static int debug_fd = -1;
+    }
-        if (debug_fd == -1) {
+    // Open Log File
-            const char *log_file_fd_env = getenv("MCPI_LOG_FILE_FD");
+    const char *file = getenv(MCPI_LOG_ENV);
-            if (log_file_fd_env == NULL) {
+    if (file == NULL) {
-                return -1;
+        file = "/dev/null";
-            }
+    }
-            debug_fd = atoi(log_file_fd_env);
+    log_fd = open(file, O_WRONLY | O_APPEND | O_CLOEXEC);
-        }
+    // Check FD
-        return debug_fd;
+    if (log_fd < 0) {
+        ERR("Unable To Open Log: %s", strerror(errno));
+    }
+    // Return
+    return reborn_get_log_fd();
+}
+void reborn_lock_log() {
+    int ret = flock(reborn_get_log_fd(), LOCK_EX);
+    if (ret != 0) {
+        ERR("Unable To Lock Log: %s", strerror(errno));
     }
 }
+void reborn_unlock_log() {
+    int ret = flock(reborn_get_log_fd(), LOCK_UN);
+    if (ret != 0) {
+        ERR("Unable To Unlock Log: %s", strerror(errno));
+    }
+}
+__attribute__((destructor)) void reborn_close_log() {
+    if (log_fd >= 0) {
+        close(log_fd);
+        log_fd = -1;
+    }
+}
+void reborn_set_log(const char *file) {
+    // Close Current Log
+    reborn_close_log();
+    // Set Variable
+    set_and_print_env(MCPI_LOG_ENV, file);
+}
+
+// Debug Logging
+static int should_print_debug_to_stderr() {
+    return getenv(MCPI_DEBUG_ENV) != NULL;
+}
+int reborn_get_debug_fd() {
+    return should_print_debug_to_stderr() ? STDERR_FILENO : reborn_get_log_fd();
+}
+void reborn_lock_debug() {
+    if (!should_print_debug_to_stderr()) {
+        reborn_lock_log();
+    }
+}
+void reborn_unlock_debug() {
+    if (!should_print_debug_to_stderr()) {
+        reborn_unlock_log();
+    }
+}