Refactor memory file opening

weichsel · Jun 9, 2024 · 8a0e219 · 8a0e219
1 parent 7b3dd0f
commit 8a0e219
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 50 deletions.
diff --git a/Sources/ZIPFoundation/Archive+BackingConfiguration.swift b/Sources/ZIPFoundation/Archive+BackingConfiguration.swift
@@ -83,14 +83,8 @@ extension Archive {
     #if swift(>=5.0)
     static func makeBackingConfiguration(for data: Data, mode: AccessMode) throws
     -> BackingConfiguration {
-        let posixMode: String
-        switch mode {
-        case .read: posixMode = "rb"
-        case .create: posixMode = "wb+"
-        case .update: posixMode = "rb+"
-        }
         let memoryFile = MemoryFile(data: data)
-        let archiveFile = memoryFile.open(mode: posixMode)
+        let archiveFile = memoryFile.open(mode: mode)
         switch mode {
         case .read:
             guard let (eocdRecord, zip64EOCD) = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {

diff --git a/Sources/ZIPFoundation/Archive+MemoryFile.swift b/Sources/ZIPFoundation/Archive+MemoryFile.swift
@@ -17,39 +17,35 @@ extension Archive {
 #if swift(>=5.0)
 
 extension Archive {
-    /// Returns a `Data` object containing a representation of the receiver.
-    public var data: Data? { return self.memoryFile?.data }
-}
 
-class MemoryFile {
+    class MemoryFile {
 
-    private(set) var data: Data
-    private var offset = 0
+        private(set) var data: Data
+        private var offset = 0
 
-    init(data: Data = Data()) {
-        self.data = data
-    }
+        init(data: Data = Data()) {
+            self.data = data
+        }
 
-    func open(mode: String) -> FILEPointer {
-        let cookie = Unmanaged.passRetained(self)
-        let writable = mode.count > 0 && (mode.first! != "r" || mode.last! == "+")
-        let append = mode.count > 0 && mode.first! == "a"
-        #if os(macOS) || os(iOS) || os(tvOS) || os(visionOS) || os(watchOS) || os(Android)
-        let result = writable
-            ? funopen(cookie.toOpaque(), readStub, writeStub, seekStub, closeStub)!
-            : funopen(cookie.toOpaque(), readStub, nil, seekStub, closeStub)!
-        #else
-        let stubs = cookie_io_functions_t(read: readStub, write: writeStub, seek: seekStub, close: closeStub)
-        let result = fopencookie(cookie.toOpaque(), mode, stubs)!
-        #endif
-        if append {
-            fseeko(result, 0, SEEK_END)
+        func open(mode: AccessMode) -> FILEPointer {
+            let cookie = Unmanaged.passRetained(self)
+            #if os(macOS) || os(iOS) || os(tvOS) || os(visionOS) || os(watchOS) || os(Android)
+            let result = mode.isWritable
+                ? funopen(cookie.toOpaque(), readStub, writeStub, seekStub, closeStub)!
+                : funopen(cookie.toOpaque(), readStub, nil, seekStub, closeStub)!
+            #else
+            let stubs = cookie_io_functions_t(read: readStub, write: writeStub, seek: seekStub, close: closeStub)
+            let result = fopencookie(cookie.toOpaque(), mode, stubs)!
+            #endif
+            return result
         }
-        return result
     }
+
+    /// Returns a `Data` object containing a representation of the receiver.
+    public var data: Data? { return self.memoryFile?.data }
 }
 
-private extension MemoryFile {
+private extension Archive.MemoryFile {
 
     func readData(buffer: UnsafeMutableRawBufferPointer) -> Int {
         let size = min(buffer.count, data.count-offset)
@@ -90,13 +86,13 @@ private extension MemoryFile {
     }
 }
 
-private func fileFromCookie(cookie: UnsafeRawPointer) -> MemoryFile {
-    return Unmanaged<MemoryFile>.fromOpaque(cookie).takeUnretainedValue()
+private func fileFromCookie(cookie: UnsafeRawPointer) -> Archive.MemoryFile {
+    return Unmanaged<Archive.MemoryFile>.fromOpaque(cookie).takeUnretainedValue()
 }
 
 private func closeStub(_ cookie: UnsafeMutableRawPointer?) -> Int32 {
     if let cookie = cookie {
-        Unmanaged<MemoryFile>.fromOpaque(cookie).release()
+        Unmanaged<Archive.MemoryFile>.fromOpaque(cookie).release()
     }
     return 0
 }

diff --git a/Sources/ZIPFoundation/Archive.swift b/Sources/ZIPFoundation/Archive.swift
@@ -102,6 +102,8 @@ public final class Archive: Sequence {
         case read
         /// Indicates that a newly instantiated `Archive` should update an existing backing file.
         case update
+
+        var isWritable: Bool { self != .read }
     }
 
     /// The version of an `Archive`

diff --git a/Tests/ZIPFoundationTests/ZIPFoundationMemoryTests.swift b/Tests/ZIPFoundationTests/ZIPFoundationMemoryTests.swift
@@ -124,7 +124,7 @@ extension ZIPFoundationTests {
     }
 
     func testReadOnlyFile() {
-        let file = MemoryFile(data: Data("ABCDEabcde".utf8)).open(mode: "r")
+        let file = Archive.MemoryFile(data: Data("ABCDEabcde".utf8)).open(mode: .read)
         var chars: [UInt8] = [0, 0, 0]
         XCTAssertEqual(fread(&chars, 1, 2, file), 2)
         XCTAssertEqual(String(Unicode.Scalar(chars[0])), "A")
@@ -144,16 +144,16 @@ extension ZIPFoundationTests {
     func testReadOnlySlicedFile() {
         let originalData = Data("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ".utf8)
         let slice = originalData[10..<originalData.count]
-        let file = MemoryFile(data: slice).open(mode: "r")
+        let file = Archive.MemoryFile(data: slice).open(mode: .read)
         var chars: [UInt8] = [0, 0, 0]
         XCTAssertEqual(fread(&chars, 1, 2, file), 2)
         XCTAssertEqual(String(Unicode.Scalar(chars[0])), "A")
         XCTAssertEqual(String(Unicode.Scalar(chars[1])), "B")
     }
 
     func testWriteOnlyFile() {
-        let mem = MemoryFile()
-        let file = mem.open(mode: "w")
+        let mem = Archive.MemoryFile()
+        let file = mem.open(mode: .create)
         XCTAssertEqual(fwrite("01234", 1, 5, file), 5)
         XCTAssertEqual(fseek(file, -2, SEEK_END), 0)
         XCTAssertEqual(fwrite("5678", 1, 4, file), 4)
@@ -163,8 +163,8 @@ extension ZIPFoundationTests {
     }
 
     func testReadWriteFile() {
-        let mem = MemoryFile(data: Data("witch".utf8))
-        let file = mem.open(mode: "r+")
+        let mem = Archive.MemoryFile(data: Data("witch".utf8))
+        let file = mem.open(mode: .update)
         XCTAssertEqual(fseek(file, 1, SEEK_CUR), 0)
         XCTAssertEqual(fwrite("a", 1, 1, file), 1)
         XCTAssertEqual(fseek(file, 0, SEEK_END), 0)
@@ -179,14 +179,6 @@ extension ZIPFoundationTests {
         XCTAssertEqual(fseek(file, 2, SEEK_SET), 0)
         XCTAssertEqual(fclose(file), 0)
     }
-
-    func testAppendFile() {
-        let mem = MemoryFile(data: Data("anti".utf8))
-        let file = mem.open(mode: "a+")
-        XCTAssertEqual(fwrite("cipation", 1, 8, file), 8)
-        XCTAssertEqual(fflush(file), 0)
-        XCTAssertEqual(mem.data, Data("anticipation".utf8))
-    }
 }
 
 // MARK: - Helpers

diff --git a/Tests/ZIPFoundationTests/ZIPFoundationTests.swift b/Tests/ZIPFoundationTests/ZIPFoundationTests.swift
@@ -325,7 +325,6 @@ extension ZIPFoundationTests {
     static var swift5OnlyTests: [(String, (ZIPFoundationTests) -> () throws -> Void)] {
         #if swift(>=5.0)
         return [
-            ("testAppendFile", testAppendFile),
             ("testCreateArchiveAddUncompressedEntryToMemory", testCreateArchiveAddUncompressedEntryToMemory),
             ("testCreateArchiveAddCompressedEntryToMemory", testCreateArchiveAddCompressedEntryToMemory),
             ("testUpdateArchiveRemoveUncompressedEntryFromMemory", testUpdateArchiveRemoveUncompressedEntryFromMemory),