Fix interaction between unread and mark (#208)

Author: nhz2

src/buffer.jl

@@ -14,7 +14,7 @@
 #   position   1   markpos     bufferpos   marginpos  lastindex(data)
 #
 # `markpos` is positive iff there are marked data; otherwise it is set to zero.
-# `markpos` ≤ `bufferpos` ≤ `marginpos` must hold whenever possible.
+# `markpos` ≤ `marginpos` and `bufferpos` ≤ `marginpos` must hold.
 
 mutable struct Buffer
     # data and positions (see above)
@@ -87,6 +87,7 @@ end
 
 function reset!(buf::Buffer)
     @assert buf.markpos > 0
+    @assert buf.markpos ≤ buf.marginpos
     buf.bufferpos = buf.markpos
     buf.markpos = 0
     return buf.bufferpos
@@ -119,7 +120,8 @@ end
 
 # Remove all buffered data.
 function emptybuffer!(buf::Buffer)
-    buf.marginpos = buf.bufferpos
+    buf.markpos = 0
+    buf.bufferpos = buf.marginpos = 1
     return buf
 end
 
@@ -133,16 +135,14 @@ function makemargin!(buf::Buffer, minsize::Integer; eager::Bool = false)
     if marginsize(buf) < minsize || eager
         # datapos refer to the leftmost position of data that must not be
         # discarded. We can left-shift to discard all data before this
-        if buf.markpos == 0
+        datapos = if iszero(buf.markpos)
             # If data is not marked we must not discard buffered (nonconsumed) data
-            datapos = buf.bufferpos
-            datasize = buffersize(buf)
+            buf.bufferpos
         else
-            # Else, we must not consume marked data
-            # (Since markpos ≤ bufferpos, we do not consume buffered data either) 
-            datapos = buf.markpos
-            datasize = buf.marginpos - buf.markpos
+            # Else, we must not consume marked or buffered data
+            min(buf.markpos, buf.bufferpos)
         end
+        datasize = buf.marginpos - datapos
         # Shift data left in buffer to make space for new data
         copyto!(buf.data, 1, buf.data, datapos, datasize)
         shift = datapos - 1
@@ -204,9 +204,21 @@ end
 # Insert data to the current buffer.
 function insertdata!(buf::Buffer, data::Ptr{UInt8}, nbytes::Integer)
     makemargin!(buf, nbytes)
-    copyto!(buf.data, buf.bufferpos + nbytes, buf.data, buf.bufferpos, buffersize(buf))
+    datapos = if iszero(buf.markpos)
+        # If data is not marked we must not discard buffered (nonconsumed) data
+        buf.bufferpos
+    else
+        # Else, we must not consume marked or buffered data
+        min(buf.markpos, buf.bufferpos)
+    end
+    datasize = buf.marginpos - datapos
+    copyto!(buf.data, datapos + nbytes, buf.data, datapos, datasize)
     GC.@preserve buf unsafe_copyto!(bufferptr(buf), data, nbytes)
     supplied!(buf, nbytes)
+    if !iszero(buf.markpos)
+        buf.markpos += nbytes
+    end
+    @assert buf.markpos ≤ buf.marginpos
     return buf
 end
 

test/codecnoop.jl

@@ -274,44 +274,72 @@
         @test !iswritable(stream)
     end
 
-    stream = NoopStream(IOBuffer(""))
-    @test TranscodingStreams.unread(stream, b"foo") === nothing
-    @test read(stream, 3) == b"foo"
-    close(stream)
+    @testset "unread" begin
+        stream = NoopStream(IOBuffer(""))
+        @test TranscodingStreams.unread(stream, b"foo") === nothing
+        @test read(stream, 3) == b"foo"
+        close(stream)
 
-    stream = NoopStream(IOBuffer("foo"))
-    @test read(stream, 3) == b"foo"
-    @test TranscodingStreams.unread(stream, b"bar") === nothing
-    @test read(stream, 3) == b"bar"
-    close(stream)
+        stream = NoopStream(IOBuffer("foo"))
+        @test read(stream, 3) == b"foo"
+        @test TranscodingStreams.unread(stream, b"bar") === nothing
+        @test read(stream, 3) == b"bar"
+        close(stream)
 
-    stream = NoopStream(IOBuffer("foobar"))
-    @test TranscodingStreams.unread(stream, b"baz") === nothing
-    @test read(stream, 3) == b"baz"
-    @test read(stream, 3) == b"foo"
-    @test read(stream, 3) == b"bar"
-    @test eof(stream)
-    close(stream)
+        stream = NoopStream(IOBuffer("foobar"))
+        @test TranscodingStreams.unread(stream, b"baz") === nothing
+        @test read(stream, 3) == b"baz"
+        @test read(stream, 3) == b"foo"
+        @test read(stream, 3) == b"bar"
+        @test eof(stream)
+        close(stream)
 
-    stream = NoopStream(IOBuffer("foobar"))
-    @test read(stream, 3) == b"foo"
-    @test TranscodingStreams.unread(stream, b"baz") === nothing
-    @test read(stream, 3) == b"baz"
-    @test read(stream, 3) == b"bar"
-    @test eof(stream)
-    close(stream)
+        stream = NoopStream(IOBuffer("foobar"))
+        @test read(stream, 3) == b"foo"
+        @test TranscodingStreams.unread(stream, b"baz") === nothing
+        @test read(stream, 3) == b"baz"
+        @test read(stream, 3) == b"bar"
+        @test eof(stream)
+        close(stream)
 
-    stream = NoopStream(IOBuffer("foobar"))
-    @test read(stream, 3) == b"foo"
-    @test read(stream, 3) == b"bar"
-    @test TranscodingStreams.unread(stream, b"baz") === nothing
-    @test read(stream, 3) == b"baz"
-    @test eof(stream)
-    close(stream)
+        stream = NoopStream(IOBuffer("foobar"))
+        @test read(stream, 3) == b"foo"
+        @test read(stream, 3) == b"bar"
+        @test TranscodingStreams.unread(stream, b"baz") === nothing
+        @test read(stream, 3) == b"baz"
+        @test eof(stream)
+        close(stream)
 
-    stream = NoopStream(IOBuffer("foobar"))
-    @test_throws ArgumentError TranscodingStreams.unsafe_unread(stream, pointer(b"foo"), -1)
-    close(stream)
+        for bufsize in (1, 2, 3, 4, 100)
+            for n in (1, 100)
+                stream = NoopStream(IOBuffer("foo"^n*"bar"^n); bufsize)
+                @test mark(stream) == 0
+                @test read(stream, 3n) == codeunits("foo"^n)
+                @test read(stream, 3n) == codeunits("bar"^n)
+                TranscodingStreams.unread(stream, codeunits("baz"^n))
+                @test reset(stream) == 0
+                @test read(stream, 3n) == codeunits("foo"^n)
+                @test read(stream, 3n) == codeunits("baz"^n)
+                @test eof(stream)
+                close(stream)
+            end
+        end
+
+        # unread before mark
+        stream = NoopStream(IOBuffer("foobar"); bufsize=16)
+        @test read(stream, String) == "foobar"
+        mark(stream)
+        for i in 1:100
+            TranscodingStreams.unread(stream, b"foo")
+        end
+        @test read(stream, String) == "foo"^100
+        @test reset(stream) == 6
+        @test eof(stream)
+
+        stream = NoopStream(IOBuffer("foobar"))
+        @test_throws ArgumentError TranscodingStreams.unsafe_unread(stream, pointer(b"foo"), -1)
+        close(stream)
+    end
 
     stream = NoopStream(IOBuffer(""))
     unsafe_write(stream, C_NULL, 0)