Remove unneeded Base.unsafe_read NoopStream method (#199)

Author: nhz2

fuzz/fuzz.jl

@@ -58,6 +58,40 @@ function wrap_stream(codecs_kws, io::IO)::IO
     end
 end
 
+# Read data using various means.
+# These function read a vector of bytes from io,
+# if io is eof they should return an empty vector.
+read_methods = Data.SampledFrom([
+    function read_byte(io)
+        eof(io) && return UInt8[]
+        [read(io, UInt8)]
+    end,
+    function read_by_readuntil_keep(io)
+        delim = 0x01
+        readuntil(io, delim; keep=true)
+    end,
+    function read_by_Base_unsafe_read(io)
+        n = bytesavailable(io)
+        ret = zeros(UInt8, n)
+        GC.@preserve ret Base.unsafe_read(io, pointer(ret), n)
+        ret
+    end,
+    function read_by_readavailable(io)
+        readavailable(io)
+    end,
+    function read_by_readbytes1(io)
+        ret = zeros(UInt8, 10000)
+        n = readbytes!(io, ret)
+        ret[1:n]
+    end,
+    function read_by_readbytes2(io)
+        ret = zeros(UInt8, 0)
+        n = readbytes!(io, ret, 10000)
+        ret[1:n]
+    end
+])
+
+
 @check function read_byte_data(
         kws=read_codecs_kws,
         data=datas,
@@ -76,6 +110,21 @@ end
     read(stream) == data || return false
     eof(stream)
 end
+@check function read_data_methods(
+        kws=read_codecs_kws,
+        data=datas,
+        rs=Data.Vectors(read_methods),
+    )
+    stream = wrap_stream(kws, IOBuffer(data))
+    x = UInt8[]
+    for r in rs
+        d = r(stream)
+        append!(x, d)
+        # TODO fix position
+        # length(x) == position(stream) || return false
+    end
+    x == data[eachindex(x)]
+end
 
 # flush all nested streams and return final data
 function take_all(stream)

src/noop.jl

@@ -97,28 +97,6 @@ function Base.seekend(stream::NoopStream)
     return stream
 end
 
-function Base.unsafe_read(stream::NoopStream, output::Ptr{UInt8}, nbytes::UInt)
-    changemode!(stream, :read)
-    buffer = stream.buffer1
-    p = output
-    p_end = output + nbytes
-    while p < p_end && !eof(stream)
-        if buffersize(buffer) > 0
-            m = min(buffersize(buffer), p_end - p)
-            copydata!(p, buffer, m)
-        else
-            # directly read data from the underlying stream
-            m = p_end - p
-            Base.unsafe_read(stream.stream, p, m)
-        end
-        p += m
-    end
-    if p < p_end && eof(stream)
-        throw(EOFError())
-    end
-    return
-end
-
 function Base.unsafe_write(stream::NoopStream, input::Ptr{UInt8}, nbytes::UInt)
     changemode!(stream, :write)
     buffer = stream.buffer1

src/stream.jl

@@ -400,7 +400,7 @@ function Base.unsafe_read(stream::TranscodingStream, output::Ptr{UInt8}, nbytes:
         p += m
         GC.safepoint()
     end
-    if p < p_end && eof(stream)
+    if p < p_end
         throw(EOFError())
     end
     return

test/codecdoubleframe.jl

@@ -284,6 +284,22 @@ DoubleFrameDecoderStream(stream::IO; kwargs...) = TranscodingStream(DoubleFrameD
         @test eof(s2)
     end
 
+    @testset "reading zero bytes from invalid stream" begin
+        # This behavior is required to avoid breaking JLD2.jl
+        # `s` must go into read mode, but not actually call `eof`
+        for readnone in (io -> read!(io, UInt8[]), io -> read(io, 0))
+            for invalid_data in (b"", b"asdf")
+                s = DoubleFrameDecoderStream(IOBuffer(invalid_data;read=true,write=true))
+                @test iswritable(s)
+                @test isreadable(s)
+                readnone(s)
+                @test !iswritable(s)
+                @test isreadable(s)
+                @test_throws ErrorException eof(s)
+            end
+        end
+    end
+
     test_roundtrip_read(DoubleFrameEncoderStream, DoubleFrameDecoderStream)
     test_roundtrip_write(DoubleFrameEncoderStream, DoubleFrameDecoderStream)
     test_roundtrip_lines(DoubleFrameEncoderStream, DoubleFrameDecoderStream)