Improve StdioOutputStream with fwrite (#180) (#188)

Replaced `fputs` with `fwrite`, added test and made spelling corrections Motivation: If a 0 byte is logged, `fputs` would not output the content after the 0 byte making it harder to debug (see #180). `fwrite` uses a count argument so content after 0 byte can be logged. Modifications: Replaced `fputs` with the correct call to `fwrite`. Added helper internal func `contiguousUTF8(_ string: String) -> String.UTF8View`. Added `testStdioOutputStreamWrite()` to test ability to log content after 0 byte. Result: Improved debugging experience Co-authored-by: Johannes Weiss <[email protected]>
apple · Apr 6, 2021 · b1d1c03 · b1d1c03
1 parent bc049d0
commit b1d1c03
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 6 deletions.
diff --git a/Sources/Logging/Logging.swift b/Sources/Logging/Logging.swift
@@ -808,7 +808,7 @@ internal struct StdioOutputStream: TextOutputStream {
     internal let flushMode: FlushMode
 
     internal func write(_ string: String) {
-        string.withCString { ptr in
+        self.contiguousUTF8(string).withContiguousStorageIfAvailable { utf8Bytes in
             #if os(Windows)
             _lock_file(self.file)
             #elseif canImport(WASILibc)
@@ -825,11 +825,11 @@ internal struct StdioOutputStream: TextOutputStream {
                 funlockfile(self.file)
                 #endif
             }
-            _ = fputs(ptr, self.file)
+            _ = fwrite(utf8Bytes.baseAddress!, 1, utf8Bytes.count, self.file)
             if case .always = self.flushMode {
                 self.flush()
             }
-        }
+        }!
     }
 
     /// Flush the underlying stream.
@@ -838,6 +838,16 @@ internal struct StdioOutputStream: TextOutputStream {
         _ = fflush(self.file)
     }
 
+    internal func contiguousUTF8(_ string: String) -> String.UTF8View {
+        var contiguousString = string
+        #if compiler(>=5.1)
+        contiguousString.makeContiguousUTF8()
+        #else
+        contiguousString = string + ""
+        #endif
+        return contiguousString.utf8
+    }
+
     internal static let stderr = StdioOutputStream(file: systemStderr, flushMode: .always)
     internal static let stdout = StdioOutputStream(file: systemStdout, flushMode: .always)
 

diff --git a/Tests/LoggingTests/LoggingTest+XCTest.swift b/Tests/LoggingTests/LoggingTest+XCTest.swift
@@ -51,6 +51,7 @@ extension LoggingTest {
             ("testStreamLogHandlerOutputFormat", testStreamLogHandlerOutputFormat),
             ("testStreamLogHandlerOutputFormatWithMetaData", testStreamLogHandlerOutputFormatWithMetaData),
             ("testStreamLogHandlerOutputFormatWithOrderedMetadata", testStreamLogHandlerOutputFormatWithOrderedMetadata),
+            ("testStdioOutputStreamWrite", testStdioOutputStreamWrite),
             ("testStdioOutputStreamFlush", testStdioOutputStreamFlush),
             ("testOverloadingError", testOverloadingError),
         ]

diff --git a/Tests/LoggingTests/LoggingTest.swift b/Tests/LoggingTests/LoggingTest.swift
@@ -725,6 +725,22 @@ class LoggingTest: XCTestCase {
         XCTAssert(interceptStream.strings[1].contains("a=a1 b=b1"))
     }
 
+    func testStdioOutputStreamWrite() {
+        self.withWriteReadFDsAndReadBuffer { writeFD, readFD, readBuffer in
+            let logStream = StdioOutputStream(file: writeFD, flushMode: .always)
+            LoggingSystem.bootstrapInternal { StreamLogHandler(label: $0, stream: logStream) }
+            let log = Logger(label: "test")
+            let testString = "hello\u{0} world"
+            log.critical("\(testString)")
+
+            let size = read(readFD, readBuffer, 256)
+
+            let output = String(decoding: UnsafeRawBufferPointer(start: UnsafeRawPointer(readBuffer), count: size), as: UTF8.self)
+            let messageSucceeded = output.trimmingCharacters(in: .whitespacesAndNewlines).hasSuffix(testString)
+            XCTAssertTrue(messageSucceeded)
+        }
+    }
+
     func testStdioOutputStreamFlush() {
         // flush on every statement
         self.withWriteReadFDsAndReadBuffer { writeFD, readFD, readBuffer in
@@ -758,7 +774,7 @@ class LoggingTest: XCTestCase {
         var fds: [Int32] = [-1, -1]
         fds.withUnsafeMutableBufferPointer { ptr in
             let err = pipe(ptr.baseAddress!)
-            XCTAssertEqual(err, 0, "pipe faild \(err)")
+            XCTAssertEqual(err, 0, "pipe failed \(err)")
         }
 
         let writeFD = fdopen(fds[1], "w")
@@ -769,11 +785,11 @@ class LoggingTest: XCTestCase {
         }
 
         var err = setvbuf(writeFD, writeBuffer, _IOFBF, 256)
-        XCTAssertEqual(err, 0, "setvbuf faild \(err)")
+        XCTAssertEqual(err, 0, "setvbuf failed \(err)")
 
         let readFD = fds[0]
         err = fcntl(readFD, F_SETFL, fcntl(readFD, F_GETFL) | O_NONBLOCK)
-        XCTAssertEqual(err, 0, "fcntl faild \(err)")
+        XCTAssertEqual(err, 0, "fcntl failed \(err)")
 
         let readBuffer = UnsafeMutablePointer<Int8>.allocate(capacity: 256)
         defer {