From c7cba01f0db80c67de1b8a76e98e9ae628a6b84e Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 5 May 2022 15:22:58 +0100 Subject: [PATCH 1/3] FileHelpers: Add FileDescriptor.read(filling buffer:) Because `read(into:)` and `write(into:)` return the number of bytes read or written which may be smaller than desired, users will often want to call these functions in a loop until they have read or written all the bytes they require. Such loops require keeping track of the index and will be repeated toil in each application. Swift System already provides an extensions `writeAll(_:)` and `writeAll(toAbsoluteOffset:_:)` which operates on a sequence of bytes and writes all the bytes in the sequence. This patch adds an analogous helper function for reading, `read(filling buffer:)` which takes an `UnsafeMutableRawBufferPointer` and reads until the buffer is full, or until EOF is reached. --- Sources/System/FileHelpers.swift | 69 ++++++++++++++++++++++ Sources/System/FileOperations.swift | 11 +++- Tests/SystemTests/FileOperationsTest.swift | 33 ++++++++++- 3 files changed, 111 insertions(+), 2 deletions(-) diff --git a/Sources/System/FileHelpers.swift b/Sources/System/FileHelpers.swift index 0825f83c..fd599f09 100644 --- a/Sources/System/FileHelpers.swift +++ b/Sources/System/FileHelpers.swift @@ -79,6 +79,75 @@ extension FileDescriptor { } } + /// Reads bytes at the current file offset into a buffer until the buffer is filled. + /// + /// - Parameters: + /// - buffer: The region of memory to read into. + /// - Returns: The number of bytes that were read, equal to `buffer.count`. + /// + /// This method either reads until `buffer` is full, or throws an error if + /// only part of the buffer was filled. + /// + /// The property of `buffer` + /// determines the number of bytes that are read into the buffer. + @_alwaysEmitIntoClient + @discardableResult + public func read( + filling buffer: UnsafeMutableRawBufferPointer + ) throws -> Int { + return try _read(filling: buffer).get() + } + + /// Reads bytes at the current file offset into a buffer until the buffer is filled. + /// + /// - Parameters: + /// - offset: The file offset where reading begins. + /// - buffer: The region of memory to read into. + /// - Returns: The number of bytes that were read, equal to `buffer.count`. + /// + /// This method either reads until `buffer` is full, or throws an error if + /// only part of the buffer was filled. + /// + /// The property of `buffer` + /// determines the number of bytes that are read into the buffer. + @_alwaysEmitIntoClient + @discardableResult + public func read( + fromAbsoluteOffset offset: Int64, + filling buffer: UnsafeMutableRawBufferPointer + ) throws -> Int { + return try _read(fromAbsoluteOffset: offset, filling: buffer).get() + } + + @usableFromInline + internal func _read( + fromAbsoluteOffset offset: Int64? = nil, + filling buffer: UnsafeMutableRawBufferPointer + ) -> Result { + var idx = 0 + while idx < buffer.count { + let readResult: Result + if let offset = offset { + readResult = _read( + fromAbsoluteOffset: offset + Int64(idx), + into: UnsafeMutableRawBufferPointer(rebasing: buffer[idx...]), + retryOnInterrupt: true + ) + } else { + readResult = _readNoThrow( + into: UnsafeMutableRawBufferPointer(rebasing: buffer[idx...]), + retryOnInterrupt: true + ) + } + switch readResult { + case .success(let numBytes): idx += numBytes + case .failure(let err): return .failure(err) + } + } + assert(idx == buffer.count) + return .success(buffer.count) + } + /// Writes a sequence of bytes to the given offset. /// /// - Parameters: diff --git a/Sources/System/FileOperations.swift b/Sources/System/FileOperations.swift index 01025632..742bddc2 100644 --- a/Sources/System/FileOperations.swift +++ b/Sources/System/FileOperations.swift @@ -158,14 +158,23 @@ extension FileDescriptor { into buffer: UnsafeMutableRawBufferPointer, retryOnInterrupt: Bool = true ) throws -> Int { - try _read(into: buffer, retryOnInterrupt: retryOnInterrupt).get() + try _readNoThrow(into: buffer, retryOnInterrupt: retryOnInterrupt).get() } + // NOTE: This function (mistakenly marked as throws) is vestigial but remains to preserve ABI. @usableFromInline internal func _read( into buffer: UnsafeMutableRawBufferPointer, retryOnInterrupt: Bool ) throws -> Result { + _readNoThrow(into: buffer, retryOnInterrupt: retryOnInterrupt) + } + + @usableFromInline + internal func _readNoThrow( + into buffer: UnsafeMutableRawBufferPointer, + retryOnInterrupt: Bool + ) -> Result { valueOrErrno(retryOnInterrupt: retryOnInterrupt) { system_read(self.rawValue, buffer.baseAddress, buffer.count) } diff --git a/Tests/SystemTests/FileOperationsTest.swift b/Tests/SystemTests/FileOperationsTest.swift index e28efd5d..20430150 100644 --- a/Tests/SystemTests/FileOperationsTest.swift +++ b/Tests/SystemTests/FileOperationsTest.swift @@ -160,5 +160,36 @@ final class FileOperationsTest: XCTestCase { issue26.runAllTests() } -} +/// This `#if` is present because, While `read(filling:)` is available on all platforms, this test +/// makes use of `FileDescriptor.pipe()` which is not available on Windows. +#if !os(Windows) + func testReadFilling() async throws { + let pipe = try FileDescriptor.pipe() + defer { + try? pipe.writeEnd.close() + try? pipe.readEnd.close() + } + var abc = "abc" + var def = "def" + let abcdef = abc + def + + try abc.withUTF8 { + XCTAssertEqual(try pipe.writeEnd.writeAll(UnsafeRawBufferPointer($0)), 3) + } + + async let readBytes = try Array(unsafeUninitializedCapacity: abcdef.count) { buf, count in + count = try pipe.readEnd.read(filling: UnsafeMutableRawBufferPointer(buf)) + XCTAssertEqual(count, abcdef.count) + } + + try def.withUTF8 { + XCTAssertEqual(try pipe.writeEnd.writeAll(UnsafeRawBufferPointer($0)), 3) + } + + let _readBytes = try await readBytes + + XCTAssertEqual(_readBytes, Array(abcdef.utf8)) + } +#endif +} From 1740da94a4a7e8a6ceb17ac7b90dcb2aa4a1d431 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 16 May 2022 17:20:33 +0100 Subject: [PATCH 2/3] fixup: read(filling:) returns on EOF Signed-off-by: Si Beaumont --- Sources/System/FileHelpers.swift | 11 ++++---- Tests/SystemTests/FileOperationsTest.swift | 30 +++++++++++++++++++++- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Sources/System/FileHelpers.swift b/Sources/System/FileHelpers.swift index fd599f09..35aa3d85 100644 --- a/Sources/System/FileHelpers.swift +++ b/Sources/System/FileHelpers.swift @@ -98,12 +98,12 @@ extension FileDescriptor { return try _read(filling: buffer).get() } - /// Reads bytes at the current file offset into a buffer until the buffer is filled. + /// Reads bytes at the current file offset into a buffer until the buffer is filled or until EOF is reached. /// /// - Parameters: /// - offset: The file offset where reading begins. /// - buffer: The region of memory to read into. - /// - Returns: The number of bytes that were read, equal to `buffer.count`. + /// - Returns: The number of bytes that were read, at most `buffer.count`. /// /// This method either reads until `buffer` is full, or throws an error if /// only part of the buffer was filled. @@ -125,7 +125,7 @@ extension FileDescriptor { filling buffer: UnsafeMutableRawBufferPointer ) -> Result { var idx = 0 - while idx < buffer.count { + loop: while idx < buffer.count { let readResult: Result if let offset = offset { readResult = _read( @@ -140,12 +140,13 @@ extension FileDescriptor { ) } switch readResult { + case .success(let numBytes) where numBytes == 0: break loop // EOF case .success(let numBytes): idx += numBytes case .failure(let err): return .failure(err) } } - assert(idx == buffer.count) - return .success(buffer.count) + assert(idx <= buffer.count) + return .success(idx) } /// Writes a sequence of bytes to the given offset. diff --git a/Tests/SystemTests/FileOperationsTest.swift b/Tests/SystemTests/FileOperationsTest.swift index 20430150..661a46a0 100644 --- a/Tests/SystemTests/FileOperationsTest.swift +++ b/Tests/SystemTests/FileOperationsTest.swift @@ -161,10 +161,38 @@ final class FileOperationsTest: XCTestCase { } + func testReadFillingFromFileEOF() async throws { + // Create a temporary file. + let tmpPath = "/tmp/\(UUID().uuidString)" + defer { + unlink(tmpPath) + } + + // Write some bytes. + var abc = "abc" + let byteCount = abc.count + let writeFd = try FileDescriptor.open(tmpPath, .readWrite, options: [.create, .truncate], permissions: .ownerReadWrite) + try writeFd.closeAfter { + try abc.withUTF8 { + XCTAssertEqual(try writeFd.writeAll(UnsafeRawBufferPointer($0)), byteCount) + } + } + + // Try and read more bytes than were written. + let readFd = try FileDescriptor.open(tmpPath, .readOnly) + try readFd.closeAfter { + let readBytes = try Array(unsafeUninitializedCapacity: byteCount + 1) { buf, count in + count = try readFd.read(filling: UnsafeMutableRawBufferPointer(buf)) + XCTAssertEqual(count, byteCount) + } + XCTAssertEqual(readBytes, Array(abc.utf8)) + } + } + /// This `#if` is present because, While `read(filling:)` is available on all platforms, this test /// makes use of `FileDescriptor.pipe()` which is not available on Windows. #if !os(Windows) - func testReadFilling() async throws { + func testReadFillingFromPipe() async throws { let pipe = try FileDescriptor.pipe() defer { try? pipe.writeEnd.close() From 440fda25f9aef4506f5b52222cd31633e62c15c2 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 20 May 2022 15:17:47 +0100 Subject: [PATCH 3/3] fixup: Docs and order of functions Signed-off-by: Si Beaumont --- Sources/System/FileHelpers.swift | 97 +++++++++++++++++--------------- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/Sources/System/FileHelpers.swift b/Sources/System/FileHelpers.swift index ed7830f9..75536011 100644 --- a/Sources/System/FileHelpers.swift +++ b/Sources/System/FileHelpers.swift @@ -33,54 +33,11 @@ extension FileDescriptor { return result } - /// Writes a sequence of bytes to the current offset - /// and then updates the offset. - /// - /// - Parameter sequence: The bytes to write. - /// - Returns: The number of bytes written, equal to the number of elements in `sequence`. - /// - /// This method either writes the entire contents of `sequence`, - /// or throws an error if only part of the content was written. - /// - /// Writes to the position associated with this file descriptor, and - /// increments that position by the number of bytes written. - /// See also ``seek(offset:from:)``. - /// - /// If `sequence` doesn't implement - /// the method, - /// temporary space will be allocated as needed. - @_alwaysEmitIntoClient - @discardableResult - public func writeAll( - _ sequence: S - ) throws -> Int where S.Element == UInt8 { - return try _writeAll(sequence).get() - } - - @usableFromInline - internal func _writeAll( - _ sequence: S - ) -> Result where S.Element == UInt8 { - sequence._withRawBufferPointer { buffer in - var idx = 0 - while idx < buffer.count { - switch _write( - UnsafeRawBufferPointer(rebasing: buffer[idx...]), retryOnInterrupt: true - ) { - case .success(let numBytes): idx += numBytes - case .failure(let err): return .failure(err) - } - } - assert(idx == buffer.count) - return .success(buffer.count) - } - } - - /// Reads bytes at the current file offset into a buffer until the buffer is filled. + /// Reads bytes at the current file offset into a buffer until the buffer is filled or until EOF is reached. /// /// - Parameters: /// - buffer: The region of memory to read into. - /// - Returns: The number of bytes that were read, equal to `buffer.count`. + /// - Returns: The number of bytes that were read, at most `buffer.count`. /// /// This method either reads until `buffer` is full, or throws an error if /// only part of the buffer was filled. @@ -95,7 +52,7 @@ extension FileDescriptor { return try _read(filling: buffer).get() } - /// Reads bytes at the current file offset into a buffer until the buffer is filled or until EOF is reached. + /// Reads bytes at the given file offset into a buffer until the buffer is filled or until EOF is reached. /// /// - Parameters: /// - offset: The file offset where reading begins. @@ -105,6 +62,8 @@ extension FileDescriptor { /// This method either reads until `buffer` is full, or throws an error if /// only part of the buffer was filled. /// + /// Unlike ``read(filling:)``, this method preserves the file descriptor's existing offset. + /// /// The property of `buffer` /// determines the number of bytes that are read into the buffer. @_alwaysEmitIntoClient @@ -146,6 +105,52 @@ extension FileDescriptor { return .success(idx) } + /// Writes a sequence of bytes to the current offset + /// and then updates the offset. + /// + /// - Parameter sequence: The bytes to write. + /// - Returns: The number of bytes written, equal to the number of elements in `sequence`. + /// + /// This method either writes the entire contents of `sequence`, + /// or throws an error if only part of the content was written. + /// + /// Writes to the position associated with this file descriptor, and + /// increments that position by the number of bytes written. + /// See also ``seek(offset:from:)``. + /// + /// This method either writes the entire contents of `sequence`, + /// or throws an error if only part of the content was written. + /// + /// If `sequence` doesn't implement + /// the method, + /// temporary space will be allocated as needed. + @_alwaysEmitIntoClient + @discardableResult + public func writeAll( + _ sequence: S + ) throws -> Int where S.Element == UInt8 { + return try _writeAll(sequence).get() + } + + @usableFromInline + internal func _writeAll( + _ sequence: S + ) -> Result where S.Element == UInt8 { + sequence._withRawBufferPointer { buffer in + var idx = 0 + while idx < buffer.count { + switch _write( + UnsafeRawBufferPointer(rebasing: buffer[idx...]), retryOnInterrupt: true + ) { + case .success(let numBytes): idx += numBytes + case .failure(let err): return .failure(err) + } + } + assert(idx == buffer.count) + return .success(buffer.count) + } + } + /// Writes a sequence of bytes to the given offset. /// /// - Parameters: