NSData: Refactor write(toFile:options) using FileHandle methods.

- Remove makeTemporaryFile(inDirectory:) in favour of using
  _NSCreateTemporaryFile(path).

- FileHandle: Add internal _write(buf:length) method to write out memory
  buffers.

- FileHandle: Add a throwing _synchronizeFile(), use it in synchronizeFile().

- FileHandle: Add private _checkFileHandle() to validate the file handle to
  simplify the validation in the public API calls.

Author: spevans

Foundation/FileHandle.swift

@@ -12,29 +12,41 @@ import CoreFoundation
 open class FileHandle : NSObject, NSSecureCoding {
 #if os(Windows)
     private var _handle: HANDLE
+
+    @available(windows, unavailable, message: "cannot perform non-owning handle to fd conversion")
+    open var fileDescriptor: Int32 {
+        NSUnsupported()
+    }
+
+    private func _checkFileHandle() {
+        precondition(_handle != INVALID_HANDLE_VALUE, "Invalid file handle")
+    }
+
 #else
     private var _fd: Int32
-#endif
-    private var _closeOnDealloc: Bool
 
-    @available(windows, unavailable,
-               message: "cannot perform non-owning handle to fd conversion")
     open var fileDescriptor: Int32 {
-#if os(Windows)
-        return -1
-#else
         return _fd
-#endif
     }
 
+    private func _checkFileHandle() {
+        precondition(_fd >= 0, "Bad file descriptor")
+    }
+#endif
+
+    private var _closeOnDealloc: Bool
+
+
     open var readabilityHandler: ((FileHandle) -> Void)? = {
       (FileHandle) -> Void in NSUnimplemented()
     }
+
     open var writeabilityHandler: ((FileHandle) -> Void)? = {
       (FileHandle) -> Void in NSUnimplemented()
     }
 
     open var availableData: Data {
+        _checkFileHandle()
         do {
             let readResult = try _readDataOfLength(Int.max, untilEOF: false)
             return readResult.toData()
@@ -44,10 +56,12 @@ open class FileHandle : NSObject, NSSecureCoding {
     }
 
     open func readDataToEndOfFile() -> Data {
+        _checkFileHandle()
         return readData(ofLength: Int.max)
     }
 
     open func readData(ofLength length: Int) -> Data {
+        _checkFileHandle()
         do {
             let readResult = try _readDataOfLength(length, untilEOF: true)
             return readResult.toData()
@@ -58,7 +72,6 @@ open class FileHandle : NSObject, NSSecureCoding {
 
     internal func _readDataOfLength(_ length: Int, untilEOF: Bool, options: NSData.ReadingOptions = []) throws -> NSData.NSDataReadResult {
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
 
         if length == 0 && !untilEOF {
           // Nothing requested, return empty response
@@ -131,7 +144,6 @@ open class FileHandle : NSObject, NSSecureCoding {
           free(buffer)
         }
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
         if length == 0 && !untilEOF {
             // Nothing requested, return empty response
             return NSData.NSDataReadResult(bytes: nil, length: 0, deallocator: nil)
@@ -202,78 +214,94 @@ open class FileHandle : NSObject, NSSecureCoding {
     }
 
     open func write(_ data: Data) {
-#if os(Windows)
-      precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
-      data.enumerateBytes() { (bytes, range, stop) in
-        do {
-          try NSData.write(toHandle: self._handle, path: nil,
-                           buf: UnsafeRawPointer(bytes.baseAddress!),
-                           length: bytes.count)
-        } catch {
-          fatalError("Write failure")
-        }
-      }
-#else
-        guard _fd >= 0 else { return }
+        _checkFileHandle()
         data.enumerateBytes() { (bytes, range, stop) in
             do {
-                try NSData.write(toFileDescriptor: self._fd, path: nil, buf: UnsafeRawPointer(bytes.baseAddress!), length: bytes.count)
+                try _write(buf: UnsafeRawPointer(bytes.baseAddress!), length: bytes.count)
             } catch {
-                fatalError("Write failure")
+                fatalError("Write failure: \(error)")
+            }
+        }
+    }
+
+    internal func _write(buf: UnsafeRawPointer, length: Int) throws {
+#if os(Windows)
+        var bytesRemaining = length
+        while bytesRemaining > 0 {
+            var bytesWritten: DWORD = 0
+            if WriteFile(handle, buf.advanced(by: length - bytesRemaining), DWORD(bytesRemaining), &bytesWritten, nil) == FALSE {
+                throw _NSErrorWithErrno(Int32(GetLastError()), reading: false, path: nil)
+            }
+            if BytesWritten == 0 {
+                throw _NSErrorWithErrno(Int32(GetLastError()), reading: false, path: nil)
+            }
+            bytesRemaining -= Int(bytesWritten)
+        }
+#else
+        var bytesRemaining = length
+        while bytesRemaining > 0 {
+            var bytesWritten = 0
+            repeat {
+#if canImport(Darwin)
+                    bytesWritten = Darwin.write(_fd, buf.advanced(by: length - bytesRemaining), bytesRemaining)
+#elseif canImport(Glibc)
+                   bytesWritten = Glibc.write(_fd, buf.advanced(by: length - bytesRemaining), bytesRemaining)
+#endif
+            } while (bytesWritten < 0 && errno == EINTR)
+            if bytesWritten <= 0 {
+                throw _NSErrorWithErrno(errno, reading: false, path: nil)
             }
+            bytesRemaining -= bytesWritten
         }
 #endif
     }
 
     // TODO: Error handling.
 
     open var offsetInFile: UInt64 {
+        _checkFileHandle()
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
         var liPointer: LARGE_INTEGER = LARGE_INTEGER(QuadPart: 0)
         if SetFilePointerEx(_handle, LARGE_INTEGER(QuadPart: 0),
                             &liPointer, DWORD(FILE_CURRENT)) == FALSE {
           fatalError("SetFilePointerEx failed")
         }
         return UInt64(liPointer.QuadPart)
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
         return UInt64(lseek(_fd, 0, SEEK_CUR))
 #endif
     }
 
     @discardableResult
     open func seekToEndOfFile() -> UInt64 {
+        _checkFileHandle()
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
         var liPointer: LARGE_INTEGER = LARGE_INTEGER(QuadPart: 0)
         if SetFilePointerEx(_handle, LARGE_INTEGER(QuadPart: 0),
                             &liPointer, DWORD(FILE_END)) == FALSE {
           fatalError("SetFilePointerEx failed")
         }
         return UInt64(liPointer.QuadPart)
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
         return UInt64(lseek(_fd, 0, SEEK_END))
 #endif
     }
 
     open func seek(toFileOffset offset: UInt64) {
+        _checkFileHandle()
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
         if SetFilePointerEx(_handle, LARGE_INTEGER(QuadPart: LONGLONG(offset)),
                             nil, DWORD(FILE_BEGIN)) == FALSE {
           fatalError("SetFilePointerEx failed")
         }
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
         lseek(_fd, off_t(offset), SEEK_SET)
 #endif
     }
 
     open func truncateFile(atOffset offset: UInt64) {
+        _checkFileHandle()
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
         if SetFilePointerEx(_handle, LARGE_INTEGER(QuadPart: LONGLONG(offset)),
                             nil, DWORD(FILE_BEGIN)) == FALSE {
           fatalError("SetFilePointerEx failed")
@@ -282,21 +310,29 @@ open class FileHandle : NSObject, NSSecureCoding {
           fatalError("SetEndOfFile failed")
         }
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
         if lseek(_fd, off_t(offset), SEEK_SET) < 0 { fatalError("lseek() failed.") }
         if ftruncate(_fd, off_t(offset)) < 0 { fatalError("ftruncate() failed.") }
 #endif
     }
 
     open func synchronizeFile() {
+        _checkFileHandle()
+        do {
+            try _synchronizeFile()
+        } catch {
+          fatalError("synchronizeFile failed: \(error)")
+        }
+    }
+
+    internal func _synchronizeFile() throws {
 #if os(Windows)
-        precondition(_handle != INVALID_HANDLE_VALUE, "invalid file handle")
         if FlushFileBuffers(_handle) == FALSE {
-          fatalError("FlushFileBuffers failed: \(GetLastError())")
+            throw _NSErrorWithErrno(Int32(GetLastError()), reading: false)
         }
 #else
-        precondition(_fd >= 0, "Bad file descriptor")
-        fsync(_fd)
+        guard fsync(_fd) == 0 else {
+            throw _NSErrorWithErrno(errno, reading: false)
+        }
 #endif
     }
 

Foundation/NSData.swift

@@ -433,100 +433,47 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
         return result
     }
 
-    internal func makeTemporaryFile(inDirectory dirPath: String) throws -> (Int32, String) {
-        let template = dirPath._nsObject.appendingPathComponent("tmp.XXXXXX")
-        let maxLength = Int(PATH_MAX) + 1
-        var buf = [Int8](repeating: 0, count: maxLength)
-        let _ = template._nsObject.getFileSystemRepresentation(&buf, maxLength: maxLength)
-        let fd = mkstemp(&buf)
-        if fd == -1 {
-            throw _NSErrorWithErrno(errno, reading: false, path: dirPath)
-        }
-        let pathResult = FileManager.default.string(withFileSystemRepresentation:buf, length: Int(strlen(buf)))
-        return (fd, pathResult)
-    }
-
-    internal class func write(toFileDescriptor fd: Int32, path: String? = nil, buf: UnsafeRawPointer, length: Int) throws {
-        var bytesRemaining = length
-        while bytesRemaining > 0 {
-            var bytesWritten : Int
-            repeat {
-                #if os(macOS) || os(iOS)
-                    bytesWritten = Darwin.write(fd, buf.advanced(by: length - bytesRemaining), bytesRemaining)
-                #elseif os(Linux) || os(Android) || CYGWIN
-                    bytesWritten = Glibc.write(fd, buf.advanced(by: length - bytesRemaining), bytesRemaining)
-                #endif
-            } while (bytesWritten < 0 && errno == EINTR)
-            if bytesWritten <= 0 {
-                throw _NSErrorWithErrno(errno, reading: false, path: path)
-            } else {
-                bytesRemaining -= bytesWritten
-            }
-        }
-    }
 
     /// Writes the data object's bytes to the file specified by a given path.
     open func write(toFile path: String, options writeOptionsMask: WritingOptions = []) throws {
-        let fm = FileManager.default
-        try fm._fileSystemRepresentation(withPath: path, { pathFsRep in
-            var fd : Int32
-            var mode : mode_t? = nil
-            let useAuxiliaryFile = writeOptionsMask.contains(.atomic)
-            var auxFilePath : String? = nil
-            if useAuxiliaryFile {
-                // Preserve permissions.
-                var info = stat()
-                if lstat(pathFsRep, &info) == 0 {
-                    let mode = mode_t(info.st_mode)
-                } else if errno != ENOENT && errno != ENAMETOOLONG {
-                    throw _NSErrorWithErrno(errno, reading: false, path: path)
-                }
-                let (newFD, path) = try self.makeTemporaryFile(inDirectory: path._nsObject.deletingLastPathComponent)
-                fd = newFD
-                auxFilePath = path
-                fchmod(fd, 0o666)
-            } else {
-                var flags = O_WRONLY | O_CREAT | O_TRUNC
-                if writeOptionsMask.contains(.withoutOverwriting) {
-                    flags |= O_EXCL
-                }
-                fd = _CFOpenFileWithMode(path, flags, 0o666)
-            }
-            if fd == -1 {
-                throw _NSErrorWithErrno(errno, reading: false, path: path)
-            }
-            defer {
-                close(fd)
-            }
 
+        func doWrite(_ fh: FileHandle) throws {
             try self.enumerateByteRangesUsingBlockRethrows { (buf, range, stop) in
                 if range.length > 0 {
-                    do {
-                        try NSData.write(toFileDescriptor: fd, path: path, buf: buf, length: range.length)
-                        if fsync(fd) < 0 {
-                            throw _NSErrorWithErrno(errno, reading: false, path: path)
-                        }
-                    } catch {
-                        if let auxFilePath = auxFilePath {
-                            try? FileManager.default.removeItem(atPath: auxFilePath)
-                        }
-                        throw error
-                    }
+                    try fh._write(buf: buf, length: range.length)
                 }
             }
-            if let auxFilePath = auxFilePath {
-                try fm._fileSystemRepresentation(withPath: auxFilePath, { auxFilePathFsRep in
-                    if rename(auxFilePathFsRep, pathFsRep) != 0 {
-                        let savedErrno = errno
-                        try? FileManager.default.removeItem(atPath: auxFilePath)
-                        throw _NSErrorWithErrno(savedErrno, reading: false, path: path)
-                    }
-                    if let mode = mode {
-                        chmod(pathFsRep, mode)
-                    }
-                })
+            try fh._synchronizeFile()
+        }
+
+        let fm = FileManager.default
+        // The destination file path may not exist so provide a default file mode of RW user only
+        let permissions = (try? fm.attributesOfItem(atPath: path)[.posixPermissions]) as? NSNumber
+        let mode = permissions?.intValue ?? 0o600
+
+        if writeOptionsMask.contains(.atomic) {
+            let (newFD, auxFilePath) = try _NSCreateTemporaryFile(path)
+            let fh = FileHandle(fileDescriptor: newFD, closeOnDealloc: true)
+            do {
+                try doWrite(fh)
+                try _NSCleanupTemporaryFile(auxFilePath, path)
+                try fm.setAttributes([.posixPermissions: NSNumber(value: mode)], ofItemAtPath: path)
+            } catch {
+                let savedErrno = errno
+                try? fm.removeItem(atPath: auxFilePath)
+                throw _NSErrorWithErrno(savedErrno, reading: false, path: path)
             }
-        })
+        } else {
+            var flags = O_WRONLY | O_CREAT | O_TRUNC
+            if writeOptionsMask.contains(.withoutOverwriting) {
+                flags |= O_EXCL
+            }
+
+            guard let fh = FileHandle(path: path, flags: flags, createMode: mode) else {
+                throw _NSErrorWithErrno(errno, reading: false, path: path)
+            }
+            try doWrite(fh)
+        }
     }
 
     /// Writes the data object's bytes to the file specified by a given path.