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: Go 2: remove io.Seeker, SeekStart, etc., change Seek method #25854

Open
ianlancetaylor opened this issue Jun 12, 2018 · 5 comments

Comments

@ianlancetaylor
Copy link
Contributor

commented Jun 12, 2018

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.

@cespare

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

commented Jun 12, 2018

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 Go2 label Jun 13, 2018

@dsnet

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.