From 7efb0b8af2f14780942080d523b2c86b142af967 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Sun, 7 Apr 2019 19:11:31 -0400 Subject: [PATCH 1/4] provide typealias for BsonPointer and MutableBsonPointer --- Sources/MongoSwift/BSON/Document.swift | 13 ++++++++----- Sources/MongoSwift/MongoCursor.swift | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Sources/MongoSwift/BSON/Document.swift b/Sources/MongoSwift/BSON/Document.swift index ea588f9ea..5b92baedf 100644 --- a/Sources/MongoSwift/BSON/Document.swift +++ b/Sources/MongoSwift/BSON/Document.swift @@ -1,9 +1,12 @@ import bson import Foundation +internal typealias MutableBsonPointer = UnsafeMutablePointer +internal typealias BsonPointer = UnsafePointer + /// The storage backing a MongoSwift `Document`. public class DocumentStorage { - internal var pointer: UnsafeMutablePointer! + internal var pointer: MutableBsonPointer! // Normally, this would go under Document, but computed properties cannot be used before all stored properties are // initialized. Putting this under DocumentStorage gives a correct count and use of it inside of an init() as long @@ -16,7 +19,7 @@ public class DocumentStorage { self.pointer = bson_new() } - internal init(fromPointer pointer: UnsafePointer) { + internal init(fromPointer pointer: BsonPointer) { self.pointer = bson_copy(pointer) } @@ -43,7 +46,7 @@ public struct Document { /// An extension of `Document` containing its private/internal functionality. extension Document { /// direct access to the storage's pointer to a bson_t - internal var data: UnsafeMutablePointer! { + internal var data: MutableBsonPointer { return storage.pointer } @@ -53,11 +56,11 @@ extension Document { * memory. * * - Parameters: - * - fromPointer: a UnsafePointer + * - fromPointer: a BsonPointer * * - Returns: a new `Document` */ - internal init(fromPointer pointer: UnsafePointer) { + internal init(fromPointer pointer: BsonPointer) { self.storage = DocumentStorage(fromPointer: pointer) self.count = self.storage.count } diff --git a/Sources/MongoSwift/MongoCursor.swift b/Sources/MongoSwift/MongoCursor.swift index f9aa0a244..308fc8bc4 100644 --- a/Sources/MongoSwift/MongoCursor.swift +++ b/Sources/MongoSwift/MongoCursor.swift @@ -79,7 +79,7 @@ public class MongoCursor: Sequence, IteratorProtocol { return err } - var replyPtr = UnsafeMutablePointer?>.allocate(capacity: 1) + var replyPtr = UnsafeMutablePointer.allocate(capacity: 1) defer { replyPtr.deallocate() } var error = bson_error_t() @@ -107,7 +107,7 @@ public class MongoCursor: Sequence, IteratorProtocol { return nil } - let out = UnsafeMutablePointer?>.allocate(capacity: 1) + let out = UnsafeMutablePointer.allocate(capacity: 1) defer { out.deinitialize(count: 1) out.deallocate() From 4f19ca0d2b12fe4064454af9433d219ee832428b Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Mon, 8 Apr 2019 07:57:23 -0400 Subject: [PATCH 2/4] make all access to `bson_t` and `bson_iter_t` explicit We often passed references of these types equally to functions that mutate the value or not, now we use the newly introduced `withBsonIterPointer` and `withMutableBsonIterPointer` --- Sources/MongoSwift/BSON/BSONValue.swift | 278 ++++++++++-------- Sources/MongoSwift/BSON/Document.swift | 26 +- .../MongoSwift/BSON/DocumentIterator.swift | 31 +- Sources/MongoSwift/BSON/Overwritable.swift | 30 +- 4 files changed, 220 insertions(+), 145 deletions(-) diff --git a/Sources/MongoSwift/BSON/BSONValue.swift b/Sources/MongoSwift/BSON/BSONValue.swift index 496f1d65b..fd21182d4 100644 --- a/Sources/MongoSwift/BSON/BSONValue.swift +++ b/Sources/MongoSwift/BSON/BSONValue.swift @@ -88,27 +88,29 @@ extension Array: BSONValue { throw wrongIterTypeError(iter, expected: Array.self) } - var length: UInt32 = 0 - let array = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - array.deinitialize(count: 1) - array.deallocate() - } - bson_iter_array(&iter.iter, &length, array) + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + let array = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + array.deinitialize(count: 1) + array.deallocate() + } + bson_iter_array(iterPtr, &length, array) - // since an array is a nested object with keys '0', '1', etc., - // create a new Document using the array data so we can recursively parse - guard let arrayData = bson_new_from_data(array.pointee, Int(length)) else { - throw RuntimeError.internalError(message: "Failed to create an Array from iterator") - } + // since an array is a nested object with keys '0', '1', etc., + // create a new Document using the array data so we can recursively parse + guard let arrayData = bson_new_from_data(array.pointee, Int(length)) else { + throw RuntimeError.internalError(message: "Failed to create an Array from iterator") + } - let arrDoc = Document(fromPointer: arrayData) + let arrDoc = Document(fromPointer: arrayData) - guard let arr = arrDoc.values as? Array else { - fatalError("Failed to cast values for document \(arrDoc) to array") - } + guard let arr = arrDoc.values as? Array else { + fatalError("Failed to cast values for document \(arrDoc) to array") + } - return arr + return arr + } } public func encode(to storage: DocumentStorage, forKey key: String) throws { @@ -260,26 +262,28 @@ public struct Binary: BSONValue, Equatable, Codable { } public static func from(iterator iter: DocumentIterator) throws -> Binary { - var subtype = bson_subtype_t(rawValue: 0) - var length: UInt32 = 0 - let dataPointer = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - dataPointer.deinitialize(count: 1) - dataPointer.deallocate() - } - guard iter.currentType == .binary else { throw wrongIterTypeError(iter, expected: Binary.self) } - bson_iter_binary(&iter.iter, &subtype, &length, dataPointer) + return try iter.withBsonIterPointer { iterPtr in + var subtype = bson_subtype_t(rawValue: 0) + var length: UInt32 = 0 + let dataPointer = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + dataPointer.deinitialize(count: 1) + dataPointer.deallocate() + } - guard let data = dataPointer.pointee else { - throw RuntimeError.internalError(message: "failed to retrieve data stored for binary BSON value") - } + bson_iter_binary(iterPtr, &subtype, &length, dataPointer) - let dataObj = Data(bytes: data, count: Int(length)) - return try self.init(data: dataObj, subtype: UInt8(subtype.rawValue)) + guard let data = dataPointer.pointee else { + throw RuntimeError.internalError(message: "failed to retrieve data stored for binary BSON value") + } + + let dataObj = Data(bytes: data, count: Int(length)) + return try self.init(data: dataObj, subtype: UInt8(subtype.rawValue)) + } } } @@ -298,7 +302,9 @@ extension Bool: BSONValue { throw wrongIterTypeError(iter, expected: Bool.self) } - return self.init(bson_iter_bool(&iter.iter)) + return iter.withBsonIterPointer { iterPtr in + self.init(bson_iter_bool(iterPtr)) + } } } @@ -326,7 +332,9 @@ extension Date: BSONValue { throw wrongIterTypeError(iter, expected: Date.self) } - return self.init(msSinceEpoch: bson_iter_date_time(&iter.iter)) + return iter.withBsonIterPointer { iterPtr in + self.init(msSinceEpoch: bson_iter_date_time(iterPtr)) + } } } @@ -362,26 +370,28 @@ public struct DBPointer: BSONValue, Codable, Equatable { } public static func from(iterator iter: DocumentIterator) throws -> DBPointer { - var length: UInt32 = 0 - let collectionPP = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - collectionPP.deinitialize(count: 1) - collectionPP.deallocate() - } + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + let collectionPP = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + collectionPP.deinitialize(count: 1) + collectionPP.deallocate() + } - let oidPP = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - oidPP.deinitialize(count: 1) - oidPP.deallocate() - } + let oidPP = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + oidPP.deinitialize(count: 1) + oidPP.deallocate() + } - bson_iter_dbpointer(&iter.iter, &length, collectionPP, oidPP) + bson_iter_dbpointer(iterPtr, &length, collectionPP, oidPP) - guard let oidP = oidPP.pointee, let collectionP = collectionPP.pointee else { - throw wrongIterTypeError(iter, expected: DBPointer.self) - } + guard let oidP = oidPP.pointee, let collectionP = collectionPP.pointee else { + throw wrongIterTypeError(iter, expected: DBPointer.self) + } - return DBPointer(ref: String(cString: collectionP), id: ObjectId(fromPointer: oidP)) + return DBPointer(ref: String(cString: collectionP), id: ObjectId(fromPointer: oidP)) + } } } @@ -450,11 +460,14 @@ public struct Decimal128: BSONValue, Equatable, Codable, CustomStringConvertible } public static func from(iterator iter: DocumentIterator) throws -> Decimal128 { - var value = bson_decimal128_t() - guard bson_iter_decimal128(&iter.iter, &value) else { - throw wrongIterTypeError(iter, expected: Decimal128.self) + return try iter.withBsonIterPointer { iterPtr in + var value = bson_decimal128_t() + guard bson_iter_decimal128(iterPtr, &value) else { + throw wrongIterTypeError(iter, expected: Decimal128.self) + } + + return Decimal128(bsonDecimal: value) } - return Decimal128(bsonDecimal: value) } } @@ -473,7 +486,9 @@ extension Double: BSONValue { throw wrongIterTypeError(iter, expected: Double.self) } - return self.init(bson_iter_double(&iter.iter)) + return iter.withBsonIterPointer { iterPtr in + self.init(bson_iter_double(iterPtr)) + } } } @@ -497,12 +512,14 @@ extension Int: BSONValue { } public static func from(iterator iter: DocumentIterator) throws -> Int { - // TODO: handle this more gracefully (SWIFT-221) - switch iter.currentType { - case .int32, .int64: - return self.init(Int(bson_iter_int32(&iter.iter))) - default: - throw wrongIterTypeError(iter, expected: Int.self) + return try iter.withBsonIterPointer { iterPtr in + // TODO: handle this more gracefully (SWIFT-221) + switch iter.currentType { + case .int32, .int64: + return self.init(Int(bson_iter_int32(iterPtr))) + default: + throw wrongIterTypeError(iter, expected: Int.self) + } } } } @@ -521,7 +538,10 @@ extension Int32: BSONValue { guard iter.currentType == .int32 else { throw wrongIterTypeError(iter, expected: Int32.self) } - return self.init(bson_iter_int32(&iter.iter)) + + return iter.withBsonIterPointer { iterPtr in + self.init(bson_iter_int32(iterPtr)) + } } } @@ -539,7 +559,10 @@ extension Int64: BSONValue { guard iter.currentType == .int64 else { throw wrongIterTypeError(iter, expected: Int64.self) } - return self.init(bson_iter_int64(&iter.iter)) + + return iter.withBsonIterPointer { iterPtr in + self.init(bson_iter_int64(iterPtr)) + } } } @@ -582,31 +605,32 @@ public struct CodeWithScope: BSONValue, Equatable, Codable { } public static func from(iterator iter: DocumentIterator) throws -> CodeWithScope { - var length: UInt32 = 0 + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + if iter.currentType.rawValue == BSONType.javascript.rawValue { + let code = String(cString: bson_iter_code(iterPtr, &length)) + return self.init(code: code) + } - if iter.currentType.rawValue == BSONType.javascript.rawValue { - let code = String(cString: bson_iter_code(&iter.iter, &length)) - return self.init(code: code) - } + guard iter.currentType == .javascriptWithScope else { + throw wrongIterTypeError(iter, expected: CodeWithScope.self) + } - guard iter.currentType == .javascriptWithScope else { - throw wrongIterTypeError(iter, expected: CodeWithScope.self) - } + var scopeLength: UInt32 = 0 + let scopePointer = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + scopePointer.deinitialize(count: 1) + scopePointer.deallocate() + } - var scopeLength: UInt32 = 0 - let scopePointer = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - scopePointer.deinitialize(count: 1) - scopePointer.deallocate() - } + let code = String(cString: bson_iter_codewscope(iterPtr, &length, &scopeLength, scopePointer)) + guard let scopeData = bson_new_from_data(scopePointer.pointee, Int(scopeLength)) else { + throw RuntimeError.internalError(message: "Failed to create a bson_t from scope data") + } + let scopeDoc = Document(fromPointer: scopeData) - let code = String(cString: bson_iter_codewscope(&iter.iter, &length, &scopeLength, scopePointer)) - guard let scopeData = bson_new_from_data(scopePointer.pointee, Int(scopeLength)) else { - throw RuntimeError.internalError(message: "Failed to create a bson_t from scope data") + return self.init(code: code, scope: scopeDoc) } - let scopeDoc = Document(fromPointer: scopeData) - - return self.init(code: code, scope: scopeDoc) } } @@ -750,10 +774,12 @@ public struct ObjectId: BSONValue, Equatable, CustomStringConvertible, Codable { } public static func from(iterator iter: DocumentIterator) throws -> ObjectId { - guard let oid = bson_iter_oid(&iter.iter) else { - throw wrongIterTypeError(iter, expected: ObjectId.self) + return try iter.withBsonIterPointer { iterPtr in + guard let oid = bson_iter_oid(iterPtr) else { + throw wrongIterTypeError(iter, expected: ObjectId.self) + } + return self.init(fromPointer: oid) } - return self.init(fromPointer: oid) } public var description: String { @@ -866,23 +892,25 @@ public struct RegularExpression: BSONValue, Equatable, Codable { } public static func from(iterator iter: DocumentIterator) throws -> RegularExpression { - let options = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - options.deinitialize(count: 1) - options.deallocate() - } + return try iter.withBsonIterPointer { iterPtr in + let options = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + options.deinitialize(count: 1) + options.deallocate() + } - guard let pattern = bson_iter_regex(&iter.iter, options) else { - throw wrongIterTypeError(iter, expected: RegularExpression.self) - } - let patternString = String(cString: pattern) + guard let pattern = bson_iter_regex(iterPtr, options) else { + throw wrongIterTypeError(iter, expected: RegularExpression.self) + } + let patternString = String(cString: pattern) - guard let stringOptions = options.pointee else { - throw RuntimeError.internalError(message: "Failed to retrieve regular expression options") - } - let optionsString = String(cString: stringOptions) + guard let stringOptions = options.pointee else { + throw RuntimeError.internalError(message: "Failed to retrieve regular expression options") + } + let optionsString = String(cString: stringOptions) - return self.init(pattern: patternString, options: optionsString) + return self.init(pattern: patternString, options: optionsString) + } } } @@ -903,19 +931,23 @@ extension String: BSONValue { } public static func from(iterator iter: DocumentIterator) throws -> String { - var length: UInt32 = 0 - guard iter.currentType == .string, let strValue = bson_iter_utf8(&iter.iter, &length) else { - throw wrongIterTypeError(iter, expected: String.self) - } + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + guard iter.currentType == .string, let strValue = bson_iter_utf8(iterPtr, &length) else { + throw wrongIterTypeError(iter, expected: String.self) + } - guard bson_utf8_validate(strValue, Int(length), true) else { - throw RuntimeError.internalError(message: "String \(strValue) not valid UTF-8") - } + guard bson_utf8_validate(strValue, Int(length), true) else { + throw RuntimeError.internalError(message: "String \(strValue) not valid UTF-8") + } - guard let out = self.init(rawStringData: strValue, length: Int(length)) else { - throw RuntimeError.internalError(message: "Underlying string data could not be parsed to a Swift String") + guard let out = self.init(rawStringData: strValue, length: Int(length)) else { + throw RuntimeError.internalError( + message: "Underlying string data could not be parsed to a Swift String") + } + + return out } - return out } } @@ -955,16 +987,18 @@ public struct Symbol: BSONValue, CustomStringConvertible, Codable, Equatable { } public static func from(iterator iter: DocumentIterator) throws -> Symbol { - var length: UInt32 = 0 - guard iter.currentType == .symbol, let cStr = bson_iter_symbol(&iter.iter, &length) else { - throw wrongIterTypeError(iter, expected: Symbol.self) - } + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + guard iter.currentType == .symbol, let cStr = bson_iter_symbol(iterPtr, &length) else { + throw wrongIterTypeError(iter, expected: Symbol.self) + } - guard let strValue = String(rawStringData: cStr, length: Int(length)) else { - throw RuntimeError.internalError(message: "Cannot parse String from underlying data") - } + guard let strValue = String(rawStringData: cStr, length: Int(length)) else { + throw RuntimeError.internalError(message: "Cannot parse String from underlying data") + } - return Symbol(strValue) + return Symbol(strValue) + } } } @@ -1008,12 +1042,14 @@ public struct Timestamp: BSONValue, Equatable, Codable { guard iter.currentType == .timestamp else { throw wrongIterTypeError(iter, expected: Timestamp.self) } - var t: UInt32 = 0 - var i: UInt32 = 0 - bson_iter_timestamp(&iter.iter, &t, &i) + return iter.withBsonIterPointer { iterPtr in + var t: UInt32 = 0 + var i: UInt32 = 0 - return self.init(timestamp: t, inc: i) + bson_iter_timestamp(iterPtr, &t, &i) + return self.init(timestamp: t, inc: i) + } } } diff --git a/Sources/MongoSwift/BSON/Document.swift b/Sources/MongoSwift/BSON/Document.swift index 5b92baedf..41ce14957 100644 --- a/Sources/MongoSwift/BSON/Document.swift +++ b/Sources/MongoSwift/BSON/Document.swift @@ -1,8 +1,8 @@ import bson import Foundation -internal typealias MutableBsonPointer = UnsafeMutablePointer internal typealias BsonPointer = UnsafePointer +internal typealias MutableBsonPointer = UnsafeMutablePointer /// The storage backing a MongoSwift `Document`. public class DocumentStorage { @@ -432,20 +432,22 @@ extension Document: BSONValue { throw wrongIterTypeError(iter, expected: Document.self) } - var length: UInt32 = 0 - let document = UnsafeMutablePointer?>.allocate(capacity: 1) - defer { - document.deinitialize(count: 1) - document.deallocate() - } + return try iter.withBsonIterPointer { iterPtr in + var length: UInt32 = 0 + let document = UnsafeMutablePointer?>.allocate(capacity: 1) + defer { + document.deinitialize(count: 1) + document.deallocate() + } - bson_iter_document(&iter.iter, &length, document) + bson_iter_document(iterPtr, &length, document) - guard let docData = bson_new_from_data(document.pointee, Int(length)) else { - throw RuntimeError.internalError(message: "Failed to create a Document from iterator") - } + guard let docData = bson_new_from_data(document.pointee, Int(length)) else { + throw RuntimeError.internalError(message: "Failed to create a Document from iterator") + } - return self.init(fromPointer: docData) + return self.init(fromPointer: docData) + } } } diff --git a/Sources/MongoSwift/BSON/DocumentIterator.swift b/Sources/MongoSwift/BSON/DocumentIterator.swift index 49ea32fb7..4536a937a 100644 --- a/Sources/MongoSwift/BSON/DocumentIterator.swift +++ b/Sources/MongoSwift/BSON/DocumentIterator.swift @@ -1,6 +1,9 @@ import Foundation import mongoc +internal typealias BsonIterPointer = UnsafePointer +internal typealias MutableBsonIterPointer = UnsafeMutablePointer + /// An iterator over the values in a `Document`. public class DocumentIterator: IteratorProtocol { /// the libbson iterator. it must be a `var` because we use it as @@ -32,17 +35,23 @@ public class DocumentIterator: IteratorProtocol { /// Advances the iterator forward one value. Returns false if there is an error moving forward /// or if at the end of the document. Returns true otherwise. internal func advance() -> Bool { - return bson_iter_next(&self.iter) + return self.withMutableBsonIterPointer { iterPtr in + bson_iter_next(iterPtr) + } } /// Moves the iterator to the specified key. Returns false if the key does not exist. Returns true otherwise. internal func move(to key: String) -> Bool { - return bson_iter_find(&self.iter, key.cString(using: .utf8)) + return self.withMutableBsonIterPointer { iterPtr in + bson_iter_find(iterPtr, key.cString(using: .utf8)) + } } /// Returns the current key. Assumes the iterator is in a valid position. internal var currentKey: String { - return String(cString: bson_iter_key(&self.iter)) + return self.withBsonIterPointer { iterPtr in + String(cString: bson_iter_key(iterPtr)) + } } /// Returns the current value. Assumes the iterator is in a valid position. @@ -56,7 +65,9 @@ public class DocumentIterator: IteratorProtocol { /// Returns the current value's type. Assumes the iterator is in a valid position. internal var currentType: BSONType { - return BSONType(rawValue: bson_iter_type(&self.iter).rawValue) ?? .invalid + return self.withBsonIterPointer { iterPtr in + BSONType(rawValue: bson_iter_type(iterPtr).rawValue) ?? .invalid + } } /// Returns the keys from the iterator's current position to the end. The iterator @@ -139,6 +150,18 @@ public class DocumentIterator: IteratorProtocol { try newValue.writeToCurrentPosition(of: self) } + /// Internal helper function for explicitly accessing the `bson_iter_t` as an unsafe pointer + internal func withBsonIterPointer(_ body: (BsonIterPointer) throws -> Result) rethrows -> Result { + return try withUnsafePointer(to: self.iter, body) + } + + /// Internal helper function for explicitly accessing the `bson_iter_t` as an unsafe mutable pointer + internal func withMutableBsonIterPointer( + _ body: (MutableBsonIterPointer) throws -> Result + ) rethrows -> Result { + return try withUnsafeMutablePointer(to: &self.iter, body) + } + private static let bsonTypeMap: [BSONType: BSONValue.Type] = [ .double: Double.self, .string: String.self, diff --git a/Sources/MongoSwift/BSON/Overwritable.swift b/Sources/MongoSwift/BSON/Overwritable.swift index 3c7463b64..977d893bb 100644 --- a/Sources/MongoSwift/BSON/Overwritable.swift +++ b/Sources/MongoSwift/BSON/Overwritable.swift @@ -14,7 +14,9 @@ internal protocol Overwritable: BSONValue { } extension Bool: Overwritable { - internal func writeToCurrentPosition(of iter: DocumentIterator) { bson_iter_overwrite_bool(&iter.iter, self) } + internal func writeToCurrentPosition(of iter: DocumentIterator) { + iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_bool(iterPtr, self) } + } } extension Int: Overwritable { @@ -30,15 +32,21 @@ extension Int: Overwritable { } extension Int32: Overwritable { - internal func writeToCurrentPosition(of iter: DocumentIterator) { bson_iter_overwrite_int32(&iter.iter, self) } + internal func writeToCurrentPosition(of iter: DocumentIterator) { + iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_int32(iterPtr, self) } + } } extension Int64: Overwritable { - internal func writeToCurrentPosition(of iter: DocumentIterator) { bson_iter_overwrite_int64(&iter.iter, self) } + internal func writeToCurrentPosition(of iter: DocumentIterator) { + iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_int64(iterPtr, self) } + } } extension Double: Overwritable { - internal func writeToCurrentPosition(of iter: DocumentIterator) { bson_iter_overwrite_double(&iter.iter, self) } + internal func writeToCurrentPosition(of iter: DocumentIterator) { + iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_double(iterPtr, self) } + } } extension Decimal128: Overwritable { @@ -46,7 +54,9 @@ extension Decimal128: Overwritable { withUnsafePointer(to: self.decimal128) { ptr in // bson_iter_overwrite_decimal128 takes in a (non-const) *decimal_128_t, so we need to pass in a mutable // pointer. no mutation of self.decimal128 should occur, however. (CDRIVER-3069) - bson_iter_overwrite_decimal128(&iter.iter, UnsafeMutablePointer(mutating: ptr)) + iter.withMutableBsonIterPointer { iterPtr in + bson_iter_overwrite_decimal128(iterPtr, UnsafeMutablePointer(mutating: ptr)) + } } } } @@ -54,18 +64,22 @@ extension Decimal128: Overwritable { extension ObjectId: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) throws { var encoded = try ObjectId.toLibBSONType(self.oid) - bson_iter_overwrite_oid(&iter.iter, &encoded) + iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_oid(iterPtr, &encoded) } } } extension Timestamp: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - bson_iter_overwrite_timestamp(&iter.iter, self.timestamp, self.increment) + iter.withMutableBsonIterPointer { iterPtr in + bson_iter_overwrite_timestamp(iterPtr, self.timestamp, self.increment) + } } } extension Date: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - bson_iter_overwrite_date_time(&iter.iter, self.msSinceEpoch) + iter.withMutableBsonIterPointer { iterPtr in + bson_iter_overwrite_date_time(iterPtr, self.msSinceEpoch) + } } } From 5ab8cb8ec131b3cfc7df00adb62fd1e9b2315a51 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Mon, 8 Apr 2019 15:40:56 -0400 Subject: [PATCH 3/4] `Bson` -> `BSON` --- Sources/MongoSwift/BSON/BSONValue.swift | 32 +++++++++---------- Sources/MongoSwift/BSON/Document.swift | 17 +++++----- .../MongoSwift/BSON/DocumentIterator.swift | 18 +++++------ Sources/MongoSwift/BSON/Overwritable.swift | 16 +++++----- Sources/MongoSwift/MongoCursor.swift | 4 +-- 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/Sources/MongoSwift/BSON/BSONValue.swift b/Sources/MongoSwift/BSON/BSONValue.swift index fd21182d4..59d12a0b1 100644 --- a/Sources/MongoSwift/BSON/BSONValue.swift +++ b/Sources/MongoSwift/BSON/BSONValue.swift @@ -88,7 +88,7 @@ extension Array: BSONValue { throw wrongIterTypeError(iter, expected: Array.self) } - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 let array = UnsafeMutablePointer?>.allocate(capacity: 1) defer { @@ -266,7 +266,7 @@ public struct Binary: BSONValue, Equatable, Codable { throw wrongIterTypeError(iter, expected: Binary.self) } - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var subtype = bson_subtype_t(rawValue: 0) var length: UInt32 = 0 let dataPointer = UnsafeMutablePointer?>.allocate(capacity: 1) @@ -302,7 +302,7 @@ extension Bool: BSONValue { throw wrongIterTypeError(iter, expected: Bool.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in self.init(bson_iter_bool(iterPtr)) } } @@ -332,7 +332,7 @@ extension Date: BSONValue { throw wrongIterTypeError(iter, expected: Date.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in self.init(msSinceEpoch: bson_iter_date_time(iterPtr)) } } @@ -370,7 +370,7 @@ public struct DBPointer: BSONValue, Codable, Equatable { } public static func from(iterator iter: DocumentIterator) throws -> DBPointer { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 let collectionPP = UnsafeMutablePointer?>.allocate(capacity: 1) defer { @@ -460,7 +460,7 @@ public struct Decimal128: BSONValue, Equatable, Codable, CustomStringConvertible } public static func from(iterator iter: DocumentIterator) throws -> Decimal128 { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var value = bson_decimal128_t() guard bson_iter_decimal128(iterPtr, &value) else { throw wrongIterTypeError(iter, expected: Decimal128.self) @@ -486,7 +486,7 @@ extension Double: BSONValue { throw wrongIterTypeError(iter, expected: Double.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in self.init(bson_iter_double(iterPtr)) } } @@ -512,7 +512,7 @@ extension Int: BSONValue { } public static func from(iterator iter: DocumentIterator) throws -> Int { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in // TODO: handle this more gracefully (SWIFT-221) switch iter.currentType { case .int32, .int64: @@ -539,7 +539,7 @@ extension Int32: BSONValue { throw wrongIterTypeError(iter, expected: Int32.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in self.init(bson_iter_int32(iterPtr)) } } @@ -560,7 +560,7 @@ extension Int64: BSONValue { throw wrongIterTypeError(iter, expected: Int64.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in self.init(bson_iter_int64(iterPtr)) } } @@ -605,7 +605,7 @@ public struct CodeWithScope: BSONValue, Equatable, Codable { } public static func from(iterator iter: DocumentIterator) throws -> CodeWithScope { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 if iter.currentType.rawValue == BSONType.javascript.rawValue { let code = String(cString: bson_iter_code(iterPtr, &length)) @@ -774,7 +774,7 @@ public struct ObjectId: BSONValue, Equatable, CustomStringConvertible, Codable { } public static func from(iterator iter: DocumentIterator) throws -> ObjectId { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in guard let oid = bson_iter_oid(iterPtr) else { throw wrongIterTypeError(iter, expected: ObjectId.self) } @@ -892,7 +892,7 @@ public struct RegularExpression: BSONValue, Equatable, Codable { } public static func from(iterator iter: DocumentIterator) throws -> RegularExpression { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in let options = UnsafeMutablePointer?>.allocate(capacity: 1) defer { options.deinitialize(count: 1) @@ -931,7 +931,7 @@ extension String: BSONValue { } public static func from(iterator iter: DocumentIterator) throws -> String { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 guard iter.currentType == .string, let strValue = bson_iter_utf8(iterPtr, &length) else { throw wrongIterTypeError(iter, expected: String.self) @@ -987,7 +987,7 @@ public struct Symbol: BSONValue, CustomStringConvertible, Codable, Equatable { } public static func from(iterator iter: DocumentIterator) throws -> Symbol { - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 guard iter.currentType == .symbol, let cStr = bson_iter_symbol(iterPtr, &length) else { throw wrongIterTypeError(iter, expected: Symbol.self) @@ -1043,7 +1043,7 @@ public struct Timestamp: BSONValue, Equatable, Codable { throw wrongIterTypeError(iter, expected: Timestamp.self) } - return iter.withBsonIterPointer { iterPtr in + return iter.withBSONIterPointer { iterPtr in var t: UInt32 = 0 var i: UInt32 = 0 diff --git a/Sources/MongoSwift/BSON/Document.swift b/Sources/MongoSwift/BSON/Document.swift index 41ce14957..0e4260977 100644 --- a/Sources/MongoSwift/BSON/Document.swift +++ b/Sources/MongoSwift/BSON/Document.swift @@ -1,12 +1,12 @@ import bson import Foundation -internal typealias BsonPointer = UnsafePointer -internal typealias MutableBsonPointer = UnsafeMutablePointer +internal typealias BSONPointer = UnsafePointer +internal typealias MutableBSONPointer = UnsafeMutablePointer /// The storage backing a MongoSwift `Document`. public class DocumentStorage { - internal var pointer: MutableBsonPointer! + internal var pointer: MutableBSONPointer! // Normally, this would go under Document, but computed properties cannot be used before all stored properties are // initialized. Putting this under DocumentStorage gives a correct count and use of it inside of an init() as long @@ -19,7 +19,7 @@ public class DocumentStorage { self.pointer = bson_new() } - internal init(fromPointer pointer: BsonPointer) { + internal init(fromPointer pointer: BSONPointer) { self.pointer = bson_copy(pointer) } @@ -28,6 +28,7 @@ public class DocumentStorage { guard let pointer = self.pointer else { return } + bson_destroy(pointer) self.pointer = nil } @@ -46,7 +47,7 @@ public struct Document { /// An extension of `Document` containing its private/internal functionality. extension Document { /// direct access to the storage's pointer to a bson_t - internal var data: MutableBsonPointer { + internal var data: MutableBSONPointer { return storage.pointer } @@ -56,11 +57,11 @@ extension Document { * memory. * * - Parameters: - * - fromPointer: a BsonPointer + * - fromPointer: a BSONPointer * * - Returns: a new `Document` */ - internal init(fromPointer pointer: BsonPointer) { + internal init(fromPointer pointer: BSONPointer) { self.storage = DocumentStorage(fromPointer: pointer) self.count = self.storage.count } @@ -432,7 +433,7 @@ extension Document: BSONValue { throw wrongIterTypeError(iter, expected: Document.self) } - return try iter.withBsonIterPointer { iterPtr in + return try iter.withBSONIterPointer { iterPtr in var length: UInt32 = 0 let document = UnsafeMutablePointer?>.allocate(capacity: 1) defer { diff --git a/Sources/MongoSwift/BSON/DocumentIterator.swift b/Sources/MongoSwift/BSON/DocumentIterator.swift index 4536a937a..b71ae9735 100644 --- a/Sources/MongoSwift/BSON/DocumentIterator.swift +++ b/Sources/MongoSwift/BSON/DocumentIterator.swift @@ -1,8 +1,8 @@ import Foundation import mongoc -internal typealias BsonIterPointer = UnsafePointer -internal typealias MutableBsonIterPointer = UnsafeMutablePointer +internal typealias BSONIterPointer = UnsafePointer +internal typealias MutableBSONIterPointer = UnsafeMutablePointer /// An iterator over the values in a `Document`. public class DocumentIterator: IteratorProtocol { @@ -35,21 +35,21 @@ public class DocumentIterator: IteratorProtocol { /// Advances the iterator forward one value. Returns false if there is an error moving forward /// or if at the end of the document. Returns true otherwise. internal func advance() -> Bool { - return self.withMutableBsonIterPointer { iterPtr in + return self.withMutableBSONIterPointer { iterPtr in bson_iter_next(iterPtr) } } /// Moves the iterator to the specified key. Returns false if the key does not exist. Returns true otherwise. internal func move(to key: String) -> Bool { - return self.withMutableBsonIterPointer { iterPtr in + return self.withMutableBSONIterPointer { iterPtr in bson_iter_find(iterPtr, key.cString(using: .utf8)) } } /// Returns the current key. Assumes the iterator is in a valid position. internal var currentKey: String { - return self.withBsonIterPointer { iterPtr in + return self.withBSONIterPointer { iterPtr in String(cString: bson_iter_key(iterPtr)) } } @@ -65,7 +65,7 @@ public class DocumentIterator: IteratorProtocol { /// Returns the current value's type. Assumes the iterator is in a valid position. internal var currentType: BSONType { - return self.withBsonIterPointer { iterPtr in + return self.withBSONIterPointer { iterPtr in BSONType(rawValue: bson_iter_type(iterPtr).rawValue) ?? .invalid } } @@ -151,13 +151,13 @@ public class DocumentIterator: IteratorProtocol { } /// Internal helper function for explicitly accessing the `bson_iter_t` as an unsafe pointer - internal func withBsonIterPointer(_ body: (BsonIterPointer) throws -> Result) rethrows -> Result { + internal func withBSONIterPointer(_ body: (BSONIterPointer) throws -> Result) rethrows -> Result { return try withUnsafePointer(to: self.iter, body) } /// Internal helper function for explicitly accessing the `bson_iter_t` as an unsafe mutable pointer - internal func withMutableBsonIterPointer( - _ body: (MutableBsonIterPointer) throws -> Result + internal func withMutableBSONIterPointer( + _ body: (MutableBSONIterPointer) throws -> Result ) rethrows -> Result { return try withUnsafeMutablePointer(to: &self.iter, body) } diff --git a/Sources/MongoSwift/BSON/Overwritable.swift b/Sources/MongoSwift/BSON/Overwritable.swift index 977d893bb..ae2c41c2c 100644 --- a/Sources/MongoSwift/BSON/Overwritable.swift +++ b/Sources/MongoSwift/BSON/Overwritable.swift @@ -15,7 +15,7 @@ internal protocol Overwritable: BSONValue { extension Bool: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_bool(iterPtr, self) } + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_bool(iterPtr, self) } } } @@ -33,19 +33,19 @@ extension Int: Overwritable { extension Int32: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_int32(iterPtr, self) } + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_int32(iterPtr, self) } } } extension Int64: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_int64(iterPtr, self) } + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_int64(iterPtr, self) } } } extension Double: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_double(iterPtr, self) } + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_double(iterPtr, self) } } } @@ -54,7 +54,7 @@ extension Decimal128: Overwritable { withUnsafePointer(to: self.decimal128) { ptr in // bson_iter_overwrite_decimal128 takes in a (non-const) *decimal_128_t, so we need to pass in a mutable // pointer. no mutation of self.decimal128 should occur, however. (CDRIVER-3069) - iter.withMutableBsonIterPointer { iterPtr in + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_decimal128(iterPtr, UnsafeMutablePointer(mutating: ptr)) } } @@ -64,13 +64,13 @@ extension Decimal128: Overwritable { extension ObjectId: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) throws { var encoded = try ObjectId.toLibBSONType(self.oid) - iter.withMutableBsonIterPointer { iterPtr in bson_iter_overwrite_oid(iterPtr, &encoded) } + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_oid(iterPtr, &encoded) } } } extension Timestamp: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_timestamp(iterPtr, self.timestamp, self.increment) } } @@ -78,7 +78,7 @@ extension Timestamp: Overwritable { extension Date: Overwritable { internal func writeToCurrentPosition(of iter: DocumentIterator) { - iter.withMutableBsonIterPointer { iterPtr in + iter.withMutableBSONIterPointer { iterPtr in bson_iter_overwrite_date_time(iterPtr, self.msSinceEpoch) } } diff --git a/Sources/MongoSwift/MongoCursor.swift b/Sources/MongoSwift/MongoCursor.swift index 308fc8bc4..233959ddb 100644 --- a/Sources/MongoSwift/MongoCursor.swift +++ b/Sources/MongoSwift/MongoCursor.swift @@ -79,7 +79,7 @@ public class MongoCursor: Sequence, IteratorProtocol { return err } - var replyPtr = UnsafeMutablePointer.allocate(capacity: 1) + var replyPtr = UnsafeMutablePointer.allocate(capacity: 1) defer { replyPtr.deallocate() } var error = bson_error_t() @@ -107,7 +107,7 @@ public class MongoCursor: Sequence, IteratorProtocol { return nil } - let out = UnsafeMutablePointer.allocate(capacity: 1) + let out = UnsafeMutablePointer.allocate(capacity: 1) defer { out.deinitialize(count: 1) out.deallocate() From bcc015e500ca41e17e8e4d1a221a17562944c3e8 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Mon, 8 Apr 2019 16:10:52 -0400 Subject: [PATCH 4/4] missed two cases where we were using `bson_iter_t` by reference --- Sources/MongoSwift/BSON/DocumentIterator.swift | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Sources/MongoSwift/BSON/DocumentIterator.swift b/Sources/MongoSwift/BSON/DocumentIterator.swift index b71ae9735..2365c39d1 100644 --- a/Sources/MongoSwift/BSON/DocumentIterator.swift +++ b/Sources/MongoSwift/BSON/DocumentIterator.swift @@ -17,7 +17,12 @@ public class DocumentIterator: IteratorProtocol { internal init?(forDocument doc: Document) { self.iter = bson_iter_t() self.storage = doc.storage - guard bson_iter_init(&self.iter, doc.data) else { + + let initialized = self.withMutableBSONIterPointer { iterPtr in + bson_iter_init(iterPtr, doc.data) + } + + guard initialized else { return nil } } @@ -27,7 +32,12 @@ public class DocumentIterator: IteratorProtocol { internal init?(forDocument doc: Document, advancedTo key: String) { self.iter = bson_iter_t() self.storage = doc.storage - guard bson_iter_init_find(&iter, doc.data, key.cString(using: .utf8)) else { + + let initialized = self.withMutableBSONIterPointer { iterPtr in + bson_iter_init_find(iterPtr, doc.data, key.cString(using: .utf8)) + } + + guard initialized else { return nil } }