Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow unread of AbstractVector{UInt8} #212

Merged
merged 3 commits into from
May 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/buffer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ end

# Make margin with ≥`minsize` and return the size of it.
# If eager is true, it tries to move data even when the buffer has enough margin.
function makemargin!(buf::Buffer, minsize::Integer; eager::Bool = false)
function makemargin!(buf::Buffer, minsize::Int; eager::Bool = false)
@assert minsize ≥ 0
if buffersize(buf) == 0 && buf.markpos == 0
buf.bufferpos = buf.marginpos = 1
Expand Down Expand Up @@ -156,7 +156,7 @@ function makemargin!(buf::Buffer, minsize::Integer; eager::Bool = false)
# At least enough for minsize, but otherwise 1.5 times
if marginsize(buf) < minsize
datasize = length(buf.data)
resize!(buf.data, max(buf.marginpos + minsize - 1, datasize + div(datasize, 2)))
resize!(buf.data, max(Base.checked_add(buf.marginpos, minsize) - 1, datasize + div(datasize, 2)))
end
@assert marginsize(buf) ≥ minsize
return marginsize(buf)
Expand Down Expand Up @@ -185,7 +185,7 @@ end

# Copy data from `data` to `buf`.
function copydata!(buf::Buffer, data::Ptr{UInt8}, nbytes::Integer)
makemargin!(buf, nbytes)
makemargin!(buf, Int(nbytes))
GC.@preserve buf unsafe_copyto!(marginptr(buf), data, nbytes)
supplied!(buf, nbytes)
return buf
Expand All @@ -202,7 +202,9 @@ function copydata!(data::Ptr{UInt8}, buf::Buffer, nbytes::Integer)
end

# Insert data to the current buffer.
function insertdata!(buf::Buffer, data::Ptr{UInt8}, nbytes::Integer)
# `data` must not alias `buf`
function insertdata!(buf::Buffer, data::Union{AbstractArray{UInt8}, Memory})
nhz2 marked this conversation as resolved.
Show resolved Hide resolved
nbytes = Int(length(data))
makemargin!(buf, nbytes)
datapos = if iszero(buf.markpos)
# If data is not marked we must not discard buffered (nonconsumed) data
Expand All @@ -213,7 +215,9 @@ function insertdata!(buf::Buffer, data::Ptr{UInt8}, nbytes::Integer)
end
datasize = buf.marginpos - datapos
copyto!(buf.data, datapos + nbytes, buf.data, datapos, datasize)
GC.@preserve buf unsafe_copyto!(bufferptr(buf), data, nbytes)
for i in 0:nbytes-1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use copyto!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyto! isn't defined for TranscodingStreams.Memory, but it does have getindex

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also copyto! seems like it has special cases for handling aliasing. JuliaLang/julia#50900
This isn't needed here.

buf.data[buf.bufferpos + i] = data[firstindex(data) + i]
end
supplied!(buf, nbytes)
if !iszero(buf.markpos)
buf.markpos += nbytes
Expand Down
4 changes: 4 additions & 0 deletions src/memory.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ end
return mem.size
end

@inline function Base.firstindex(mem::Memory)
return 1
end

@inline function Base.lastindex(mem::Memory)
return Int(mem.size)
end
Expand Down
2 changes: 1 addition & 1 deletion src/noop.jl
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function Base.unsafe_write(stream::NoopStream, input::Ptr{UInt8}, nbytes::UInt):
end
buffer = stream.buffer1
if marginsize(buffer) ≥ nbytes
copydata!(buffer, input, nbytes)
copydata!(buffer, input, Int(nbytes))
return Int(nbytes)
else
flushbuffer(stream)
Expand Down
15 changes: 10 additions & 5 deletions src/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -442,15 +442,18 @@ function Base.readavailable(stream::TranscodingStream)
end

"""
unread(stream::TranscodingStream, data::Vector{UInt8})
unread(stream::TranscodingStream, data::AbstractVector{UInt8})

Insert `data` to the current reading position of `stream`.

The next `read(stream, sizeof(data))` call will read data that are just
inserted.

`data` must not alias any internal buffers in `stream`
"""
function unread(stream::TranscodingStream, data::ByteData)
GC.@preserve data unsafe_unread(stream, pointer(data), sizeof(data))
function unread(stream::TranscodingStream, data::AbstractVector{UInt8})
insertdata!(stream.buffer1, data)
return nothing
end

"""
Expand All @@ -460,13 +463,15 @@ Insert `nbytes` pointed by `data` to the current reading position of `stream`.

The data are copied into the internal buffer and hence `data` can be safely used
after the operation without interfering the stream.

`data` must not alias any internal buffers in `stream`
"""
function unsafe_unread(stream::TranscodingStream, data::Ptr, nbytes::Integer)
if nbytes < 0
throw(ArgumentError("negative nbytes"))
end
ready_to_read!(stream)
insertdata!(stream.buffer1, convert(Ptr{UInt8}, data), nbytes)
insertdata!(stream.buffer1, Memory(convert(Ptr{UInt8}, data), UInt(nbytes)))
return nothing
end

Expand Down Expand Up @@ -702,7 +707,7 @@ function callprocess(stream::TranscodingStream, inbuf::Buffer, outbuf::Buffer)
if stream.state.mode == :read
if stream.stream isa TranscodingStream && !has_sharedbuf(stream) && !iszero(buffersize(inbuf))
# unread data to match behavior if inbuf was shared.
GC.@preserve inbuf unsafe_unread(stream.stream, bufferptr(inbuf), buffersize(inbuf))
unread(stream.stream, view(inbuf.data, inbuf.bufferpos:inbuf.marginpos-1))
end
changemode!(stream, :stop)
end
Expand Down
2 changes: 2 additions & 0 deletions test/Project.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
[deps]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
FillArrays = "1a297f60-69ca-5386-bcde-b61e274b549b"
OffsetArrays = "6fe1bfb0-de20-5000-8ca7-80f57d26f881"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
TranscodingStreams = "3bb67fe8-82b1-5028-8e26-92a6c54297fa"
29 changes: 29 additions & 0 deletions test/codecnoop.jl
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
using OffsetArrays: OffsetArray
using FillArrays: Zeros

@testset "Noop Codec" begin
source = IOBuffer("")
stream = TranscodingStream(Noop(), source)
Expand Down Expand Up @@ -339,6 +342,32 @@
stream = NoopStream(IOBuffer("foobar"))
@test_throws ArgumentError TranscodingStreams.unsafe_unread(stream, pointer(b"foo"), -1)
close(stream)

stream = NoopStream(IOBuffer("foo"))
@test read(stream, 3) == b"foo"
@test TranscodingStreams.unread(stream, OffsetArray(b"bar", -5:-3)) === nothing
@test read(stream, 3) == b"bar"
close(stream)

stream = NoopStream(IOBuffer("foobar"))
@test read(stream, 3) == b"foo"
@test_throws OverflowError TranscodingStreams.unread(stream, Zeros{UInt8}(typemax(Int))) === nothing
close(stream)

stream = NoopStream(IOBuffer("foo"))
@test read(stream, 3) == b"foo"
@test TranscodingStreams.unread(stream, Zeros{UInt8}(big(3))) === nothing
@test read(stream, 3) == b"\0\0\0"
close(stream)

stream = NoopStream(IOBuffer("foo"))
@test read(stream, 3) == b"foo"
d = b"bar"
GC.@preserve d begin
@test TranscodingStreams.unsafe_unread(stream, pointer(d), 3) === nothing
end
@test read(stream, 3) == b"bar"
close(stream)
end

stream = NoopStream(IOBuffer(""))
Expand Down
Loading