Skip to content

Commit

Permalink
Fix unsafe_read for IOBuffer with non dense data (#55776)
Browse files Browse the repository at this point in the history
Fixes one part of #54636 

It was only safe to use the following if `from.data` was a dense vector
of bytes.
```julia
GC.@preserve from unsafe_copyto!(p, pointer(from.data, from.ptr), adv)
```

This PR adds a fallback suggested by @matthias314 in
https://discourse.julialang.org/t/copying-bytes-from-abstractvector-to-ptr/119408/7
  • Loading branch information
nhz2 authored Oct 25, 2024
1 parent e8aacbf commit ec2e121
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
21 changes: 20 additions & 1 deletion base/iobuffer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,33 @@ function unsafe_read(from::GenericIOBuffer, p::Ptr{UInt8}, nb::UInt)
from.readable || _throw_not_readable()
avail = bytesavailable(from)
adv = min(avail, nb)
GC.@preserve from unsafe_copyto!(p, pointer(from.data, from.ptr), adv)
unsafe_read!(p, from.data, from.ptr, adv)
from.ptr += adv
if nb > avail
throw(EOFError())
end
nothing
end

function unsafe_read!(dest::Ptr{UInt8}, src::AbstractVector{UInt8}, so::Integer, nbytes::UInt)
for i in 1:nbytes
unsafe_store!(dest, @inbounds(src[so+i-1]), i)
end
end

# Note: Currently, CodeUnits <: DenseVector, which makes this union redundant w.r.t
# DenseArrayType{UInt8}, but this is a bug, and may be removed in future versions
# of Julia. See #54002
const DenseBytes = Union{
<:DenseArrayType{UInt8},
CodeUnits{UInt8, <:Union{String, SubString{String}}},
}

function unsafe_read!(dest::Ptr{UInt8}, src::DenseBytes, so::Integer, nbytes::UInt)
GC.@preserve src unsafe_copyto!(dest, pointer(src, so), nbytes)
nothing
end

function peek(from::GenericIOBuffer, T::Union{Type{Int16},Type{UInt16},Type{Int32},Type{UInt32},Type{Int64},Type{UInt64},Type{Int128},Type{UInt128},Type{Float16},Type{Float32},Type{Float64}})
from.readable || _throw_not_readable()
avail = bytesavailable(from)
Expand Down
8 changes: 0 additions & 8 deletions base/strings/search.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,6 @@ function findnext(pred::Fix2{<:Union{typeof(isequal),typeof(==)},<:AbstractChar}
end
end

# Note: Currently, CodeUnits <: DenseVector, which makes this union redundant w.r.t
# DenseArrayType{UInt8}, but this is a bug, and may be removed in future versions
# of Julia. See #54002
const DenseBytes = Union{
<:DenseArrayType{UInt8},
CodeUnits{UInt8, <:Union{String, SubString{String}}},
}

function findfirst(pred::Fix2{<:Union{typeof(isequal),typeof(==)},<:Union{UInt8, Int8}}, a::Union{DenseInt8, DenseUInt8})
findnext(pred, a, firstindex(a))
end
Expand Down
10 changes: 10 additions & 0 deletions test/iobuffer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -389,3 +389,13 @@ end
b = pushfirst!([0x02], 0x01)
@test take!(IOBuffer(b)) == [0x01, 0x02]
end

@testset "#54636 reading from non-dense vectors" begin
data = 0x00:0xFF
io = IOBuffer(data)
@test read(io) == data

data = @view(collect(0x00:0x0f)[begin:2:end])
io = IOBuffer(data)
@test read(io) == data
end

0 comments on commit ec2e121

Please sign in to comment.