Fixes for position with nested NoopStreams
#203
Conversation
| elseif mode === :write | ||
| return position(stream.stream) + buffersize(stream.buffer1) | ||
| elseif mode === :read | ||
| else # read | ||
| return position(stream.stream) - buffersize(stream.buffer1) |
There was a problem hiding this comment.
If buffer1 is shared by stream.stream and stream this would double count buffersize
| elseif mode === :idle | ||
| return 0 | ||
| elseif has_sharedbuf(stream) | ||
| return position(stream.stream) |
There was a problem hiding this comment.
Here I treat the buffer as being "owned" by the underlying stream even though it is being shared. This is also why I need to add special cases for writing to a NoopStream that shares its buffer with the underlying stream. The underlying stream must be notified to change from :idle to :write mode before its buffer1 is changed.
| @@ -53,16 +53,18 @@ Note that this method may return a wrong position when | |||
| - some data have been inserted by `TranscodingStreams.unread`, or | |||
| - the position of the wrapped stream has been changed outside of this package. | |||
| """ | |||
| function Base.position(stream::NoopStream) | |||
| function Base.position(stream::NoopStream)::Int64 | |||
There was a problem hiding this comment.
I'm trying to decide if this should be Int64 or Int. The latter varies based on the system word size.
$ julia +1.10~x86 -E "typeof(0)"
Int32
There was a problem hiding this comment.
Cool, I didn't know juliaup had 32-bit julia.
This should be Int64 to support large files.
| changemode!(stream, :write) | ||
| if has_sharedbuf(stream) | ||
| # directly write data to the underlying stream | ||
| n = Int(write(stream.stream, b)) |
There was a problem hiding this comment.
Why do you need the explicit cast to Int here?
There was a problem hiding this comment.
I need this to fix the stats function for NoopStream, but that's for a future PR.
There was a problem hiding this comment.
Right now the cast isn't doing anything.
| changemode!(stream, :write) | ||
| if has_sharedbuf(stream) | ||
| # directly write data to the underlying stream | ||
| n = Int(unsafe_write(stream.stream, input, nbytes)) |
There was a problem hiding this comment.
Why do you need the explicit cast to Int here?
With this PR the basic fuzzing tests for
positionwhile reading finally pass.