-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Description
The doc comment of std.Io.Reader.VTable.stream states:
/// Although this function is usually called when `buffer` is empty, it is
/// also called when it needs to be filled more due to the API user
/// requesting contiguous memory. In either case, the existing buffer data
/// should be ignored; new data written to `w`.
This seems to be suggesting that it is valid for the std.Io.Reader interface to fill up its buffer by streaming to a fixed std.Io.Writer which fills the remaining buffer capacity. However, the doc comment also states:
/// In addition to, or instead of writing to `w`, the implementation may
/// choose to store data in `buffer`, modifying `seek` and `end`
/// accordingly. Implementations are encouraged to take advantage of
/// this if it simplifies the logic.
This clearly states that it is valid for an implementation to stream data to w and put some more data in the buffer.
These two conditions cannot be reconciled. If the Reader implementation uses stream as the first part suggests, but the implementation decides to also buffer some data, then the n bytes streamed to w will fill r.buffer[r.end..][0..n], and the m bytes written to the buffer will fill r.buffer[r.end..][0..m]---immediately clobbering those bytes. We need to change the rules here: either stream cannot interact with the buffer, or (more likely) writing to w cannot modify the buffer.
As mentioned above, the doc comment of std.Io.Reader.VTable.stream states that it may choose to store data in buffer, modifying seek and end. In this case, it is unclear whether it is allowed for it to "undo" a rebase: moving the seek from 0 to some later index.
The same is unclear for std.Io.Reader.VTable.readVec. Its doc comment states:
/// Implementations may ignore `data`, writing directly to `Reader.buffer`,
/// modifying `seek` and `end` accordingly, and returning 0 from this
/// function. Implementations are encouraged to take advantage of this if
/// it simplifies the logic.
From this comment, it is not even clear that an implementation is allowed to populate both data and buffer---though of course we already have ones that do, such as fs.File.Reader. Assuming that an implementation may indeed populate both data and buffer, it is again not mentioned whether or not the implementation is allowed to "undo" a rebase by advancing seek.
So, for both stream and readVec, we must as the null hypothesis assume that the implementation may advance seek past the buffer start. Yet, parts of the interface such as fill rely on this not being the case, because they rebase once and then try to use these functions to populate the buffer.
It is unclear how std.Io.Reader.VTable.stream behaves when error.WriteFailed occurs. Is the Reader still in a valid (usable) state? If so, are the bytes which failed to write lost, or is the implementation required to make sure they're kept (be it in the buffer or the implementation state)?