TranscodingStreams.jl icon indicating copy to clipboard operation
TranscodingStreams.jl copied to clipboard

safety improvement to Memory

Open vtjnash opened this issue 1 year ago • 3 comments

While Memory was in theory a slightly safer wrapper for unsafe_read / unsafe_write operations, it doesn't appear to have every needed to be used that way, and ways just took in Vector{UInt8}. That seems safer to keep it that way therefore.

Closes https://github.com/JuliaIO/TranscodingStreams.jl/issues/125

vtjnash avatar Feb 02 '24 04:02 vtjnash

If I want to use the "transcoding protocol" to decompress data directly into a Matrix{Float64} I would use unsafe_wrap to convert the matrix to a Vector{UInt8} in this new version of the API? I don't fully understand what the warning in the docstring of unsafe_wrap means and if this is safe to do:

the programmer is responsible also for ensuring that the underlying data is not accessed through two arrays of different element type, similar to the strict aliasing rule in C.

nhz2 avatar Feb 02 '24 20:02 nhz2

It is generally not a good idea to use unsafe_wrap on existing julia memory. It is really primarily for C memory only.

However, the IO protocol in Base is mostly based around calling unsafe_load/unsafe_store!/unsafe_copyto! because of the reasons you stated, about wanting to do IO directly to and from arbitrary types. The Memory representation might even be an okay way to represent doing that, as a means of creating a fat pointer for adding memory-safety to IO. But it didn't seem like this package was testing any of that.

vtjnash avatar Feb 02 '24 23:02 vtjnash

But anyways, that documentation is sort of a lie there, since that is not really how anything seems to be implemented here. Nor would it be even be that sensible to implement that way. Everything is required to go through the Buffer object (a Vector) and only that object ever gets converted to Memory (basically, a slower, less-safe view of said Buffer). For example: https://github.com/JuliaIO/TranscodingStreams.jl/blob/d7e83709e1fa6733920f1736413c1c7d7df4d8f3/src/stream.jl#L681-L697 or c.f. sloweof implementation

vtjnash avatar Feb 03 '24 05:02 vtjnash