Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: io/v2: remove io.Seeker, SeekStart, etc., change Seek method #25854

Open
ianlancetaylor opened this issue Jun 12, 2018 · 5 comments
Open
Labels
Proposal Proposal-Hold v2 A language change or incompatible library change
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

Copying @bradfitz's comment #17920 (comment) into a separate proposal.

For Go 2 we should consider changing the Seek method to just take a file position, not a whence argument. We should consider adding SeekFromCurrent and SeekFromEnd, though I suspect they are unnecessary. We should change Seeker similarly. We should remove io.SeekStart, io,SeekCurrent, and io.SeekEnd.

Rationale: the current Seek method is copied from C. It presents three different interfaces, and doesn't make much sense as a single method in Go. People who implement the io.Seeker interface often only implement it for io.SeekStart, and do not correctly handle the other possible values.

See also #17920, which this would replace.

@ianlancetaylor ianlancetaylor added Proposal NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 12, 2018
@ianlancetaylor ianlancetaylor added this to the Proposal milestone Jun 12, 2018
@cespare
Copy link
Contributor

cespare commented Jun 12, 2018

We should consider adding SeekFromCurrent and SeekFromEnd, though I suspect they are unnecessary.

I looked through all my own code and the Go code at my company.

  • Most uses of Seek are indeed SeekStart.
  • We've never used SeekCurrent.
  • There are a handful of uses of SeekEnd. Most of these are in functions which want to take a single io.ReadSeeker and only call Seek(0, io.SeekEnd) once, at the beginning, to learn the file size. (http.ServeContent does this too.) Perhaps this could be changed in Go 2 to use some standard interface for file-like things that know their size.

@bradfitz
Copy link
Contributor

SeekCurrent is generally only used with 0 to learn the current seek position, and often just to restore it at the end of some operation that really wanted an io.ReaderAt and is hoping that restoring it afterwards won't be too racy.

@mvdan mvdan added the v2 A language change or incompatible library change label Jun 13, 2018
@dsnet
Copy link
Member

dsnet commented Jun 14, 2018

I support the proposal to reduce the Seek method. That would avoid nasty hacks like this.

That said, I wouldn't discount use-cases for SeekEnd and SeekCurrent. If we provide other interfaces that returned the current offset and the total offset more idiomatically, then the equivalent of SeekEnd and SeekCurrent can be implemented by the user with a little bit of math.

Roughly speaking:

f.Seek(n, SeekStart)   => f.Seek(n)
f.Seek(n, SeekCurrent) => f.Seek(f.CurrentOffset()+n)
f.Seek(n, SeekEnd)     => f.Seek(f.EndOffset()+n)

EndOffset is just the filesize and can be named Size or something. It is a common piece of information needed with the ReaderAt interface (see #15822).

I much rather see idiomatic methods to access the current offset and end offset than separate SeekFromCurrent and SeekFromEnd methods.

@networkimprov
Copy link

networkimprov commented Sep 11, 2018

os.File is an API to the OS. Second-guessing the OS model for that API could have unintended consequences. If you must change os.File.Seek, please

a) rename os.File.Seek to .SeekFrom, and
b) add os.File.SeekTo(n) (or .SeekPos?) as an alias for .SeekFrom(n, io.SeekStart).

For an append-only file with an index at file's end, I use seek-from-end to read the index size, then seek-from-current to read the index. Thanks!

Edit:
@dsnet, wouldn't os.File.CurrentOffset & .Size entail a separate syscall?

@bcmills
Copy link
Contributor

bcmills commented Sep 17, 2018

hoping that restoring it afterwards won't be too racy.

If the underlying io.Reader is guaranteed to be unique, then code of that form can use a sync.Mutex or similar synchronization to ensure that it won't be racy at all.

@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2019
@ianlancetaylor ianlancetaylor changed the title proposal: Go 2: remove io.Seeker, SeekStart, etc., change Seek method proposal: io/v2: remove io.Seeker, SeekStart, etc., change Seek method Aug 23, 2023
@ianlancetaylor ianlancetaylor added Proposal-Hold and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Hold v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

8 participants