What you have is probably ok in common usage; but there is a chance that it would not properly do a copy on write change as you expect if the Data was somehow obtained from a multi-threaded environment. Here is a slight refactor (with a few extra fixes and comments for you that might save you some time later)
import Foundation import Compression public enum CompressionAlgorithm { case lz4 // speed is critical case lz4a // space is critical case zlib // reasonable speed and space case lzfse // better speed and space } private enum CompressionOperation { case compression case decompression } private func perform(_ operation: CompressionOperation, on input: Data, using algorithm: CompressionAlgorithm, workingBufferSize: Int = 2000) -> Data? { // set the algorithm let streamAlgorithm: compression_algorithm switch algorithm { case .lz4: streamAlgorithm = COMPRESSION_LZ4 case .lz4a: streamAlgorithm = COMPRESSION_LZMA case .zlib: streamAlgorithm = COMPRESSION_ZLIB case .lzfse: streamAlgorithm = COMPRESSION_LZFSE } // set the stream operation, and flags let streamOperation: compression_stream_operation let flags: Int32 switch operation { case .compression: streamOperation = COMPRESSION_STREAM_ENCODE flags = Int32(COMPRESSION_STREAM_FINALIZE.rawValue) case .decompression: streamOperation = COMPRESSION_STREAM_DECODE flags = 0 } let dstSize = workingBufferSize let dstPointer = UnsafeMutablePointer<UInt8>.allocate(capacity: dstSize) // you could technically use a Data for this too by accessing the mutable bytes ;) defer { dstPointer.deallocate(capacity: dstSize) } let inputCount = input.count return input.withUnsafeBytes { (bytes: UnsafePointer<UInt8>) in // the closure here can return a Data so the entire compression operation can be encapsulated in the lambda var output = Data() var stream = compression_stream(dst_ptr: dstPointer, dst_size: dstSize, src_ptr: bytes, src_size: inputCount, state: nil) var status = compression_stream_init(&stream, streamOperation, streamAlgorithm) guard status != COMPRESSION_STATUS_ERROR else { return nil // perhaps instead of making this nullable a throw? } defer { compression_stream_destroy(&stream) } while status == COMPRESSION_STATUS_OK { // process the stream status = compression_stream_process(&stream, flags) // collect bytes from the stream and reset switch status { case COMPRESSION_STATUS_OK: output.append(dstPointer, count: dstSize) stream.dst_ptr = dstPointer stream.dst_size = dstSize break // I am certain you probably meant to break here? case COMPRESSION_STATUS_ERROR: return nil // perhaps instead of making this nullable a throw? case COMPRESSION_STATUS_END: output.append(dstPointer, count: stream.dst_ptr - dstPointer) break // here too, this should break right? default: break // this probably should be a fatal error since it is an unexpected result. } } return output } } // Compressed keeps the compressed data and the algorithm // together as one unit, so you never forget how the data was // compressed. public struct Compressed { public let data: Data // this was what was saving your state of the pointer public let algorithm: CompressionAlgorithm public init(data: Data, algorithm: CompressionAlgorithm) { self.data = data self.algorithm = algorithm } // Compress the input with the specified algorith. Returns nil if it fails. public static func compress(input: Data, with algorithm: CompressionAlgorithm) -> Compressed? { guard let data = perform(.compression, on: input, using: algorithm) else { return nil } return Compressed(data: data, algorithm: algorithm) } // Factory method to return uncompressed data. Returns nil if it cannot be decompressed. public func makeDecompressed() -> Data? { return perform(.decompression, on: data, using: algorithm) } } // For discoverability, add a compressed method to Data extension Data { // Factory method to make compressed data or nil if it fails. public func makeCompressed(with algorithm: CompressionAlgorithm) -> Compressed? { return Compressed.compress(input: self, with: algorithm) } } // Example usage: let input = Data(bytes: Array(repeating: UInt8(123), count: 10000)) let compressed = input.makeCompressed(with: .lzfse) compressed?.data.count // in most cases much less than orginal input count let restoredInput = compressed?.makeDecompressed() input == restoredInput // true > On Dec 9, 2016, at 9:04 AM, Ray Fix <ray...@gmail.com> wrote: > > Hi Philippe, > > Thank you for your response! > >> On Dec 9, 2016, at 8:44 AM, Philippe Hausler <phaus...@apple.com >> <mailto:phaus...@apple.com>> wrote: >> >> So letting the bytes escape from the closure could potentially be in that >> territory of “bad things may happen”. If you know for certain that the Data >> will not be mutated and it’s underlying storage will last for the duration >> of the usage then you should be able to get away with it. >> > > So far it is looking like I am getting away with it. I believe the conditions > that you state hold. > >> The bytes pointer is an inner pointer return from the encapsulating object >> backing the struct Data. Since that backing object can potentially be >> deallocated via mutation or the buffer may be reallocated (which very often >> can cause the base address of the pointer to change) we decided it would be >> safer to use a closure to ensure the lifespan of the bytes over the access >> to the bytes. >> >> Perhaps it might be best to understand more of what you are trying to do to >> offer a better solution. > > I am trying to use the compression_stream C API in the Compression framework > and wrap it so it is a bit more Swifty. :] > > A full gist of the playground is here: > https://gist.github.com/rayfix/a286bb55accffef09249ba3535993782 > <https://gist.github.com/rayfix/a286bb55accffef09249ba3535993782> > > See line 91. > > Appreciate the hints. > > Ray
_______________________________________________ swift-users mailing list swift-users@swift.org https://lists.swift.org/mailman/listinfo/swift-users