Make Zlib.Compressor/Decompressor classes #1769
Conversation
Motivation: z_stream stores a pointer to itself in its internal state which it checks against in inflate/deflate. As we hold these within structs, and call through to C functions which take a pointer to a z_stream, this address can change as the struct is copied about. This results in errors when calling deflate/inflate. Modifications: - Make Compressor/Decompressor classes - Move the initialize call to init and the end call to deinit. Result: Harder to misue compressor/decompressor
| /// must ``initialize()`` the compressor before using it. | ||
| struct Compressor { | ||
| /// This compressor is only suitable for compressing whole messages at a time. | ||
| final class Compressor { |
There was a problem hiding this comment.
Could we use a ~Copyable type here instead?
There was a problem hiding this comment.
I don't think that'd solve the problem as the types can still be moved around.
| // a pointer back to itself within its internal state. Calls to deflate and inflate do a state | ||
| // check ensuring that the stream pointer and its pointer-to-self held by its internal state | ||
| // are the same. However, within a struct held on the stack this location can change and results | ||
| // in errors in deflate/inflate. |
There was a problem hiding this comment.
This isn't really enough. Swift doesn't entirely promise that this will do what we want: in particular, it's really easy for Swift to accidentally copy the type. If we want a stable address, we really need to store the z_stream object inside a pointer. That opens up @FranzBusch's proposal for a ~Copyable type, assuming we have sufficient support in older Swift versions.
We do then also want to be a bit careful to re-spell all our extension methods on z_stream as instead being extension methods on UnsafeMutablePointer<z_stream>.
There was a problem hiding this comment.
Yeah that makes sense, the semantics are closer to what we actually want. Using a class felt like a bit of a hack.
Motivation: z_stream stores a pointer to itself in its internal state which it checks against in inflate/deflate. As we hold these within structs, and call through to C functions which take a pointer to a z_stream, this address can change as the struct is copied about. This results in errors when calling deflate/inflate. Modifications: - Hold a pointer to the z_stream Result: Harder to misue compressor/decompressor
4 participants
Motivation:
z_stream stores a pointer to itself in its internal state which it checks against in inflate/deflate. As we hold these within structs, and call through to C functions which take a pointer to a z_stream, this address can change as the struct is copied about. This results in errors when calling deflate/inflate.
Modifications:
Result:
Harder to misue compressor/decompressor