io: add SectionReader.Outer method

[CAFxX]

Update, Sep 6: The current proposal is #61870 (comment)

---

To support sendfile on SectionReader I propose exposing the r and base fields of SectionReader, that would thus become:

type SectionReader struct {
	// R is the parent ReaderAt this SectionReader reads from.
	R ReaderAt

	// Base is the offset (relative to R) at which the section starts.
	Base int64

	// unexported fields omitted
}

The other two fields (off and limit) do not need to be exposed (at least for the use-case above) because they can be obtained (albeit in a slightly round-about way) using Seek.

Exposing Base as described above will likely require some adjustments to the internal logic to ensure that Base is valid every time it is used internally by SectionReader (as it may have been modified between calls). Changing the Base would not affect the absolute position of the end of the section (i.e. changing the Base would change the length of the section, not its end position). Similarly, it would not affect the absolute position of the current read position, apart from the case in which changing the Base to be past the current read position would cause the Read to start returning errOffset until a subsequent Seek operation moves the read position to a valid one (or the Base is changed again to an offset not greater than the read position). Given the additional foot guns, we may want to add a comment discouraging users from manually changing Base and instead constructing a new SectionReader.

It is worth noting that enabling the mutability of both fields is not a goal of this proposal, merely a side effect of the design. As an alternative, Base() int64 could be a SectionReader method returning r.base - something that would avoid the foot guns mentioned above.

[rsc]

I think we want to change the parameter names to NewSectionReader:

func NewSectionReader(r ReaderAt, base, size int64) *SectionReader

And then add two new methods Base() int64 and Size() int64.

Do I have that right?

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[CAFxX]

@rsc using exported methods is definitely a valid alternative as well (see also the other ones), although with a small note:

And then add two new methods Base() int64 and Size() int64.

The two methods that would be required are Parent() io.ReaderAt and Base() int64. Size is already implemented, and Offset can be emulated using Seek(0, SeekCurrent) so we probably don't need it right away (at least for the sendfile use case).

To summarize, the alternate minimal method-based proposal would be:

func NewSectionReader(r ReaderAt, base, size int64) *SectionReader { /* ... */ }

func (r *SectionReader) Parent() ReaderAt { return r.r }
func (r *SectionReader) Base() int64 { return r.base }

and, if we instead want to enable fetching all offsets without modifying the current offset with Seek, we need to further add one more method:

func (r *SectionReader) Offset() int64 { return r.off-r.base }

[AlexanderYastrebov]

Maybe

func (s *SectionReader) ReaderAt() ReaderAt { return s.r }

instead of Parent()?

[CAFxX]

While I'm open to consider other names, SectionReader itself Is a ReaderAt... so calling the method ReaderAt doesn't really communicate what the method does (return the parent? or itself as a ReaderAt interface? or itself as a ReaderAt but not a Reader?)

[rsc]

It sounds like maybe just exporting the fields would be easiest. We'd want to rename the parameter to NewSectionReader from 'off, n' to 'base, size' (keeping 'n'), and then

type SectionReader struct {
	R     ReaderAt  // underlying reader
	Base  int64  // base in R where SectionReader's offset 0 is
	Size int64  // maximum SectionReader offset
	Off   int64 // current Seek offset in SectionReader (not in R), normally in [0, Size].
}

The current s.limit would be s.Base+s.Size, so some other code will need to be adjusted (there should be no unexported fields).

Do I have that right?

[CAFxX]

@rsc AFAIK that won't work as-is, because SectionReader already has a Size() method, that would clash with the proposed Size field

[ianlancetaylor]

Ouch. I guess we could call it N to more-or-less parallel LimitedReader. Size is a better name but I guess it's unavailable.

[CAFxX]

Not a huge fan of having two ways (N and Size(), Off and Seek()) for doing the same thing, but if this is the consensus it will do.

[ianlancetaylor]

Yeah, it's also not great to have a mix of exported and unexported fields.

[CAFxX]

Just to sumamrize where we are:

Current state

func NewSectionReader(r ReaderAt, off, n int64) *SectionReader

type SectionReader struct {}

func (s *SectionReader) Read(p []byte) (n int, err error)
func (s *SectionReader) Seek(offset int64, whence int) (int64, error)
func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error)
func (s *SectionReader) Size() int64

The two options below satisfy these criteria:

• No mixing of exported/unexported fields
• No go:linkname shenanigans
• Expose enough to solve net: support zero-copy send on TCPConn when reading from File via SectionReader #61727

Option 1 - Expose the fields

func NewSectionReader(r ReaderAt, base, n int64) *SectionReader

type SectionReader struct {
	R     ReaderAt // NEW
	Base  int64    // NEW
	Off   int64    // NEW (~ Seek(0, SeekCurrent))
	N     int64    // NEW (~ Size() or Seek(0, SeekEnd))
}

func (s *SectionReader) Read(p []byte) (n int, err error)
func (s *SectionReader) Seek(offset int64, whence int) (int64, error)
func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error)
func (s *SectionReader) Size() int64

• Pro: more functionalities: all fields are writable
• Con: more failure modes (Read/ReadAt can now return errOffset)
• Con: more ways to do the same thing
• Con: naming of the n/size argument to NewSectionReader a bit awkward
• Con: internal implementation likely a bit more complex

Option 2 - Expose methods

func NewSectionReader(parent ReaderAt, base, size int64) *SectionReader

type SectionReader struct {}

func (s *SectionReader) Read(p []byte) (n int, err error)
func (s *SectionReader) Seek(offset int64, whence int) (int64, error)
func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error)
func (s *SectionReader) Size() int64      // (~ Seek(0, SeekEnd))
func (s *SectionReader) Base() int64      // NEW
func (s *SectionReader) Parent() ReaderAt // NEW

• Con: less functionalities: internal state can't be written to
• Pro: same failure modes and implementation as the current ones

[rsc]

Parent seems odd since this is not a tree. Outer seems fine. And what if it returns the exact arguments that were passed to NewSectionReader, so that NewSectionReader(sr.Outer()) gets you exactly the same section reader (except for having an independent seek offset when using Read).

func NewSectionReader(parent ReaderAt, base, size int64) *SectionReader

type SectionReader struct {}

func (s *SectionReader) Read(p []byte) (n int, err error)
func (s *SectionReader) Seek(offset int64, whence int) (int64, error)
func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error)
func (s *SectionReader) Size() int64      // (~ Seek(0, SeekEnd))
func (s *SectionReader) Outer() (r ReaderAt, off, n int64)      // NEW

[CAFxX]

SGTM

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/526855 mentions this issue: io: add (*SectionReader).Outer()

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group