Rewrite Logger

This reverts all of Roop's changes to the C code, and then rewrites the
logger logic to be cleaner.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

6 files changed, 86 insertions, 114 deletions

diff --git a/WireGuard/Shared/Logging/Logger.swift b/WireGuard/Shared/Logging/Logger.swift
index 7df162a..f8ef70a 100644
--- a/WireGuard/Shared/Logging/Logger.swift
+++ b/WireGuard/Shared/Logging/Logger.swift
@@ -4,65 +4,65 @@
 import Foundation
 import os.log
 
-class Logger {
-    static var logPtr: UnsafeMutablePointer<log>?
+public class Logger {
+    static var global: Logger?
 
-    static func configure(withFilePath filePath: String) -> Bool {
-        let logPtr = filePath.withCString { filePathCStr -> UnsafeMutablePointer<log>? in
-            return open_log(filePathCStr)
+    var log: OpaquePointer?
+    var tag: String
+
+    init(withFilePath filePath: String, withTag tag: String) {
+        self.tag = tag
+        self.log = filePath.withCString { fileC -> OpaquePointer? in
+            open_log(fileC)
+        }
+        if self.log == nil {
+            os_log("Cannot open log file for writing. Log will not be saved to file.", log: OSLog.default, type: .error)
         }
-        Logger.logPtr = logPtr
-        return (logPtr != nil)
     }
 
-    static func writeLog(mergedWith otherLogFile: String, tag: String, otherTag: String, to targetFile: String) -> Bool {
-        let otherlogPtr = otherLogFile.withCString { otherLogFileCStr -> UnsafeMutablePointer<log>? in
-            return open_log(otherLogFileCStr)
+    func log(message: String) {
+        guard let log = log else { return }
+        String(format: "[%@] %@", tag, message.trimmingCharacters(in: .newlines)).withCString { messageC in
+            write_msg_to_log(log, messageC)
         }
-        if let thisLogPtr = Logger.logPtr, let otherlogPtr = otherlogPtr {
-            return targetFile.withCString { targetFileCStr -> Bool in
-                return tag.withCString { tagCStr -> Bool in
-                    return otherTag.withCString { otherTagCStr -> Bool in
-                        let returnValue = write_logs_to_file(targetFileCStr, tagCStr, thisLogPtr, otherTagCStr, otherlogPtr)
-                        return (returnValue == 0)
-                    }
-                }
-            }
+    }
+
+    func writeLog(mergedWith otherLogFile: String, to targetFile: String) -> Bool {
+        guard let log = log else { return false }
+        guard let other = otherLogFile.withCString({ otherC -> OpaquePointer? in
+            return open_log(otherC)
+        }) else { return false }
+        defer { close_log(other) }
+        return targetFile.withCString { fileC -> Bool in
+            return write_logs_to_file(fileC, log, other) == 0
         }
-        return false
     }
-}
 
-func wg_log_versions_to_file() {
-    var appVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown version"
-    if let appBuild = Bundle.main.infoDictionary?["CFBundleVersion"] as? String {
-        appVersion += " (\(appBuild))"
+    static func configureGlobal(withFilePath filePath: String?, withTag tag: String) {
+        if Logger.global != nil {
+            return
+        }
+        guard let filePath = filePath else {
+            os_log("Unable to determine log destination path. Log will not be saved to file.", log: OSLog.default, type: .error)
+            return
+        }
+        Logger.global = Logger(withFilePath: filePath, withTag: tag)
+        var appVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "Unknown version"
+        if let appBuild = Bundle.main.infoDictionary?["CFBundleVersion"] as? String {
+            appVersion += " (\(appBuild))"
+        }
+        let goBackendVersion = WIREGUARD_GO_VERSION
+        Logger.global?.log(message: "App version: \(appVersion); Go backend version: \(goBackendVersion)")
+
     }
-    let goBackendVersion = WIREGUARD_GO_VERSION
-    file_log(message: "App version: \(appVersion); Go backend version: \(goBackendVersion)")
 }
 
 func wg_log(_ type: OSLogType, staticMessage msg: StaticString) {
-    // Write to os log
     os_log(msg, log: OSLog.default, type: type)
-    // Write to file log
-    let msgString: String = msg.withUTF8Buffer { (ptr: UnsafeBufferPointer<UInt8>) -> String in
-        return String(decoding: ptr, as: UTF8.self)
-    }
-    file_log(message: msgString)
+    Logger.global?.log(message: "\(msg)")
 }
 
 func wg_log(_ type: OSLogType, message msg: String) {
-    // Write to os log
     os_log("%{public}s", log: OSLog.default, type: type, msg)
-    // Write to file log
-    file_log(message: msg)
-}
-
-private func file_log(message: String) {
-    message.withCString { messageCStr in
-        if let logPtr = Logger.logPtr {
-            write_msg_to_log(logPtr, messageCStr)
-        }
-    }
+    Logger.global?.log(message: msg)
 }
diff --git a/WireGuard/Shared/Logging/ringlogger.c b/WireGuard/Shared/Logging/ringlogger.c
index 5c6fae6..e3dfe1a 100644
--- a/WireGuard/Shared/Logging/ringlogger.c
+++ b/WireGuard/Shared/Logging/ringlogger.c
@@ -16,6 +16,23 @@
 #include <sys/mman.h>
 #include "ringlogger.h"
 
+enum {
+	MAX_LOG_LINE_LENGTH = 512,
+	MAX_LINES = 1024,
+	MAGIC = 0xdeadbeefU
+};
+
+struct log_line {
+	struct timeval tv;
+	char line[MAX_LOG_LINE_LENGTH];
+};
+
+struct log {
+	struct { uint32_t first, len; } header;
+	struct log_line lines[MAX_LINES];
+	uint32_t magic;
+};
+
 void write_msg_to_log(struct log *log, const char *msg)
 {
 	struct log_line *line = &log->lines[(log->header.first + log->header.len) % MAX_LINES];
@@ -29,13 +46,6 @@ void write_msg_to_log(struct log *log, const char *msg)
 	strncpy(line->line, msg, MAX_LOG_LINE_LENGTH - 1);
 	line->line[MAX_LOG_LINE_LENGTH - 1] = '\0';
 
-    // Trim trailing newlines
-    unsigned long length = strlen(msg);
-    while ((length > 0) && (msg[length - 1] == '\n' || msg[length - 1] == '\r')) {
-        line->line[length - 1] = '\0';
-        length--;
-    }
-
 	msync(&log->header, sizeof(log->header), MS_ASYNC);
 	msync(line, sizeof(*line), MS_ASYNC);
 }
@@ -44,12 +54,12 @@ static bool first_before_second(const struct log_line *line1, const struct log_l
 {
 	if (line1->tv.tv_sec <= line2->tv.tv_sec)
 		return true;
-	else if (line1->tv.tv_sec == line2->tv.tv_sec)
+	if (line1->tv.tv_sec == line2->tv.tv_sec)
 		return line1->tv.tv_usec <= line2->tv.tv_usec;
 	return false;
 }
 
-int write_logs_to_file(const char *file_name, const char *tag1, const struct log *log1, const char *tag2, const struct log *log2)
+int write_logs_to_file(const char *file_name, const struct log *log1, const struct log *log2)
 {
 	uint32_t i1, i2, len1 = log1->header.len, len2 = log2->header.len;
 	char buf[MAX_LOG_LINE_LENGTH];
@@ -68,22 +78,19 @@ int write_logs_to_file(const char *file_name, const char *tag1, const struct log
 		const struct log_line *line1 = &log1->lines[(log1->header.first + i1) % MAX_LINES];
 		const struct log_line *line2 = &log2->lines[(log2->header.first + i2) % MAX_LINES];
 		const struct log_line *line;
-		const char *tag;
 
 		if (i1 < len1 && (i2 >= len2 || first_before_second(line1, line2))) {
 			line = line1;
-			tag = (const char *) tag1;
 			++i1;
 		} else if (i2 < len2 && (i1 >= len1 || first_before_second(line2, line1))) {
 			line = line2;
-			tag = (const char *) tag2;
 			++i2;
 		} else {
 			break;
 		}
 		memcpy(buf, line->line, MAX_LOG_LINE_LENGTH);
 		buf[MAX_LOG_LINE_LENGTH - 1] = '\0';
-		if (fprintf(file, "%lu.%06d: [%s] %s\n", line->tv.tv_sec, line->tv.tv_usec, tag, buf) < 0) {
+		if (fprintf(file, "%lu.%06d: %s\n", line->tv.tv_sec, line->tv.tv_usec, buf) < 0) {
 			int ret = -errno;
 			fclose(file);
 			return ret;
@@ -102,10 +109,10 @@ struct log *open_log(const char *file_name)
 	if (fd < 0)
 		return NULL;
 	if (ftruncate(fd, sizeof(*log)))
-		return NULL;
+		goto err;
 	log = mmap(NULL, sizeof(*log), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 	if (log == MAP_FAILED)
-		return NULL;
+		goto err;
 	close(fd);
 
 	if (log->magic != MAGIC) {
@@ -115,4 +122,13 @@ struct log *open_log(const char *file_name)
 	}
 
 	return log;
+
+err:
+	close(fd);
+	return NULL;
+}
+
+void close_log(struct log *log)
+{
+	munmap(log, sizeof(*log));
 }
diff --git a/WireGuard/Shared/Logging/ringlogger.h b/WireGuard/Shared/Logging/ringlogger.h
index be1d33c..ad58fb8 100644
--- a/WireGuard/Shared/Logging/ringlogger.h
+++ b/WireGuard/Shared/Logging/ringlogger.h
@@ -6,25 +6,10 @@
 #ifndef RINGLOGGER_H
 #define RINGLOGGER_H
 
-enum {
-    MAX_LOG_LINE_LENGTH = 512,
-    MAX_LINES = 1024,
-    MAGIC = 0xdeadbeefU
-};
-
-struct log_line {
-    struct timeval tv;
-    char line[MAX_LOG_LINE_LENGTH];
-};
-
-struct log {
-    struct { uint32_t first, len; } header;
-    struct log_line lines[MAX_LINES];
-    uint32_t magic;
-};
-
+struct log;
 void write_msg_to_log(struct log *log, const char *msg);
-int write_logs_to_file(const char *file_name, const char *tag1, const struct log *log1, const char *tag2, const struct log *log2);
+int write_logs_to_file(const char *file_name, const struct log *log1, const struct log *log2);
 struct log *open_log(const char *file_name);
+void close_log(struct log *log);
 
 #endif
diff --git a/WireGuard/WireGuard/UI/iOS/AppDelegate.swift b/WireGuard/WireGuard/UI/iOS/AppDelegate.swift
index a5856e0..32c1286 100644
--- a/WireGuard/WireGuard/UI/iOS/AppDelegate.swift
+++ b/WireGuard/WireGuard/UI/iOS/AppDelegate.swift
@@ -12,17 +12,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 
     func application(_ application: UIApplication,
                      willFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
-
-        if let appLogFilePath = FileManager.appLogFileURL?.path {
-            if !Logger.configure(withFilePath: appLogFilePath) {
-                os_log("Can't open log file for writing. Log is not saved to file.", log: OSLog.default, type: .error)
-            }
-        } else {
-            os_log("Can't obtain log file URL. Log is not saved to file.", log: OSLog.default, type: .error)
-        }
-
-        wg_log(.info, message: "Launching app")
-        wg_log_versions_to_file()
+        Logger.configureGlobal(withFilePath: FileManager.appLogFileURL?.path, withTag: "APP")
 
         let window = UIWindow(frame: UIScreen.main.bounds)
         window.backgroundColor = UIColor.white
diff --git a/WireGuard/WireGuard/UI/iOS/SettingsTableViewController.swift b/WireGuard/WireGuard/UI/iOS/SettingsTableViewController.swift
index 2d17224..af9893d 100644
--- a/WireGuard/WireGuard/UI/iOS/SettingsTableViewController.swift
+++ b/WireGuard/WireGuard/UI/iOS/SettingsTableViewController.swift
@@ -115,13 +115,13 @@ class SettingsTableViewController: UITableViewController {
             }
 
             guard let networkExtensionLogFilePath = FileManager.networkExtensionLogFileURL?.path else {
-                ErrorPresenter.showErrorAlert(title: "Log export failed", message: "Internal error obtaining extension log path", from: self)
+                ErrorPresenter.showErrorAlert(title: "Log export failed", message: "Unable to determine extension log path", from: self)
                 return
             }
 
-            let isWritten = Logger.writeLog(mergedWith: networkExtensionLogFilePath, tag: "APP", otherTag: "EXT", to: destinationURL.path)
+            let isWritten = Logger.global?.writeLog(mergedWith: networkExtensionLogFilePath, to: destinationURL.path) ?? false
             guard isWritten else {
-                ErrorPresenter.showErrorAlert(title: "Log export failed", message: "Internal error merging logs", from: self)
+                ErrorPresenter.showErrorAlert(title: "Log export failed", message: "Unable to write logs to file", from: self)
                 return
             }
 
diff --git a/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift b/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift
index 03da7bb..65400e0 100644
--- a/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift
+++ b/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift
@@ -13,8 +13,6 @@ enum PacketTunnelProviderError: Error {
     case coultNotSetNetworkSettings
 }
 
-private var logFileHandle: FileHandle?
-
 /// A packet tunnel provider object.
 class PacketTunnelProvider: NEPacketTunnelProvider {
 
@@ -45,13 +43,9 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
 
     func startTunnel(with tunnelConfiguration: TunnelConfiguration, completionHandler startTunnelCompletionHandler: @escaping (Error?) -> Void) {
 
-        // Configure logging
         configureLogger()
 
         wg_log(.info, message: "Starting tunnel '\(tunnelConfiguration.interface.name)'")
-        wg_log_versions_to_file()
-
-        // Resolve endpoint domains
 
         let endpoints = tunnelConfiguration.peers.map { $0.endpoint }
         var resolvedEndpoints = [Endpoint?]()
@@ -141,25 +135,13 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
         if let handle = wgHandle {
             wgTurnOff(handle)
         }
-        if let fileHandle = logFileHandle {
-            fileHandle.closeFile()
-        }
         completionHandler()
     }
 
     private func configureLogger() {
-
-        // Setup writing the log to a file
-        if let networkExtensionLogFilePath = FileManager.networkExtensionLogFileURL?.path {
-            if !Logger.configure(withFilePath: networkExtensionLogFilePath) {
-                os_log("Can't open log file for writing. Log is not saved to file.", log: OSLog.default, type: .error)
-            }
-        } else {
-            os_log("Can't obtain log file URL. Log is not saved to file.", log: OSLog.default, type: .error)
-        }
-
-        // Setup WireGuard logger
-        wgSetLogger { level, msgCStr in
+        Logger.configureGlobal(withFilePath: FileManager.networkExtensionLogFileURL?.path, withTag: "EXT")
+        wgSetLogger { level, msgC in
+            guard let msgC = msgC else { return }
             let logType: OSLogType
             switch level {
             case 0:
@@ -171,8 +153,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
             default:
                 logType = .default
             }
-            let msg = (msgCStr != nil) ? String(cString: msgCStr!) : ""
-            wg_log(logType, message: msg)
+            wg_log(logType, message: String(cString: msgC))
         }
     }