code review 4344062: os: add Seek whence constants

os: add Seek whence constants

[bradfitz, 2011-04-04 17:06:03]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[bradfitz, 2011-04-04 17:09:42]

If this is cool I can update callers in this CL too.

This is minor but I always look it up, double-checking myself.  I think
constants would make for more readable code at the Seek call sites.

On Mon, Apr 4, 2011 at 10:06 AM, <bradfitz@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg
>
>
> Description:
> os: add Seek whence constants
>
> Please review this at http://codereview.appspot.com/4344062/
>
> Affected files:
>  M src/pkg/os/file.go
>
>
> Index: src/pkg/os/file.go
> ===================================================================
> --- a/src/pkg/os/file.go
> +++ b/src/pkg/os/file.go
> @@ -61,6 +61,13 @@
>        O_CREATE   int = O_CREAT            // create a new file if none
> exists.
>  )
>
> +// Seek whence values.
> +const (
> +       SEEK_ORIGIN   int = 0 // seek relative to the origin of the file
> +       SEEK_RELATIVE int = 1 // seek relative to the current offset
> +       SEEK_END      int = 2 // seek relative to the end
> +)
> +
>  type eofError int
>
>  func (eofError) String() string { return "EOF" }
>
>
>

[r, 2011-04-04 17:13:00]

People keep asking for these. I guess 0, 1, and 2 are too hard to remember.
They're certainly not too hard to type.

Russ? Anyone?

By the way, we should probably create an os.Create function because the
community has made Open too hard to use.

[bradfitz, 2011-04-04 17:22:24]

On Mon, Apr 4, 2011 at 10:13 AM, <r@golang.org> wrote:

> People keep asking for these. I guess 0, 1, and 2 are too hard to
> remember. They're certainly not too hard to type.
>

I would rather type more and have my code be readable to others who might
not have memorized some magic constants from rarely-used APIs.

But I can make my own constants in my own files if necessary.

Either way.


> Russ? Anyone?
>
> By the way, we should probably create an os.Create function because the
> community has made Open too hard to use.


You joke, but I also cringe at all those useless zeros in the common case of
opening files for reading.

Reading and writing are separate actions.  Go inherited C's API (~the
kernel's API) but it doesn't have to, imho.

[r2, 2011-04-04 17:23:08]

I was just rereading a piece of the John Lions Unix book this morning.  Perhaps
my knowledge runs too deep.

Two things bother me though:

1) The way all the constants in os are defined (repeated int = ) should maybe be
cleaned up.
2) The names are horrible throughout the os constant space.  Not sure we can do
much about that; it's silly to change the names from the underlying system. But
blech.

That said, seek(3) does not name any constants. They're 0, 1, and 2.  Moreover,
the names in open(3) differ from ours; Go's have O_ on every one. Blech again.
How did Go get worse names? Probably my fault, and worth fixing, maybe in the
add-Create pass.

-rob

[r2, 2011-04-04 17:23:57]

On Apr 4, 2011, at 10:22 AM, Brad Fitzpatrick wrote:

> 
> By the way, we should probably create an os.Create function because the
> community has made Open too hard to use.
> 
> You joke, but I also cringe at all those useless zeros in the common case of
opening files for reading.

That was not a joke, it's truth.

-rob

[bradfitz, 2011-04-04 17:28:35]

On Mon, Apr 4, 2011 at 10:23 AM, Rob 'Commander' Pike <r@google.com> wrote:
...

>
> That said, seek(3) does not name any constants. They're 0, 1, and 2.
>  Moreover, the names in open(3) differ from ours; Go's have O_ on every one.


You seem concerned with consistency with C.   Why?

[r2, 2011-04-04 17:36:09]

On Apr 4, 2011, at 10:28 AM, Brad Fitzpatrick wrote:

> On Mon, Apr 4, 2011 at 10:23 AM, Rob 'Commander' Pike <r@google.com> wrote:
> ...
> 
> That said, seek(3) does not name any constants. They're 0, 1, and 2. 
Moreover, the names in open(3) differ from ours; Go's have O_ on every one.
> 
> You seem concerned with consistency with C.   Why?

That's not the issue. The issue is that the constants are not Gopherish and
they're needlessly long, longer than even their C equivalents.

-rob

[rsc1, 2011-04-04 19:08:24]

No.

These aren't even the right names.

Russ

[bradfitz, 2011-04-04 19:22:07]

I don't care about the names.  I just made them up to start the
conversation.

Do you object to adding constants, though?


On Mon, Apr 4, 2011 at 12:08 PM, Russ Cox <rsc@google.com> wrote:

> No.
>
> These aren't even the right names.
>
> Russ
>

[rsc1, 2011-04-04 19:36:15]

On Mon, Apr 4, 2011 at 15:22, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> I don't care about the names.  I just made them up to start the
> conversation.

There are three common 0, 1, 2 in Unix:
the basic open modes, the seek whences,
and the standard file descriptors.

Somehow I missed the day when everyone
forgot that the seek whences were 0, 1, 2
and started using the names only, like in
http://linux.die.net/man/2/lseek
In that sense I guess it is not that different
from open. http://linux.die.net/man/2/open

This comes up over and over.  After comparing
http://codesearch.google.com/codesearch?q=seek\(.*,\+[012]\)
http://codesearch.google.com/codesearch?q=seek\(.*,\+SEEK_(SET|END|CUR)\)
it looks like I am definitely in the minority.
I give in.

Please add them using the standard names and type int,
so that they cannot be swapped with the offset
(same trick we use in the current Open).

I am happy to see that I am still in the majority for
http://codesearch.google.com/codesearch?q=(read|write)\([012]
http://codesearch.google.com/codesearch?q=(read|write)\(FILENO_

Russ

[bradfitz, 2011-04-04 19:40:18]

Hello golang-dev@googlegroups.com, r, r2, rsc1 (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-04-04 19:41:22]

LGTM

[bradfitz, 2011-04-04 19:46:01]

Should I update call sites in this CL?

Generally convenience CLs do, but I get the sense that you guys might prefer
0, 1, 2.

On Mon, Apr 4, 2011 at 12:41 PM, <rsc@golang.org> wrote:

> LGTM
>
>
>
> http://codereview.appspot.com/4344062/
>

[r2, 2011-04-04 19:47:01]

On Apr 4, 2011, at 12:45 PM, Brad Fitzpatrick wrote:

> Should I update call sites in this CL?
> 
> Generally convenience CLs do, but I get the sense that you guys might prefer
0, 1, 2.

Might as well. I don't promise to use the new constants everywhere, but Seeks
are pretty rare.

-rob

[rsc1, 2011-04-04 19:54:00]

Sure.  Should be a matter of

gofmt -r 'x.Seek(y, 0) -> x.Seek(y, os.SEEK_SET)'
etc

Russ

[jacek.masiulaniec_gmail.com, 2011-04-04 20:07:30]

> I am happy to see that I am still in the majority for
> http://codesearch.google.com/codesearch?q=(read|write)\([012]
> http://codesearch.google.com/codesearch?q=(read|write)\(FILENO_

You mean
http://codesearch.google.com/codesearch?q=(read|write)\(STD(IN|OUT|ERR)_FILENO

[rsc1, 2011-04-04 20:22:46]

On Mon, Apr 4, 2011 at 16:07, Jacek Masiulaniec
<jacek.masiulaniec@gmail.com> wrote:
>> I am happy to see that I am still in the majority for
>> http://codesearch.google.com/codesearch?q=(read|write)\([012]
>> http://codesearch.google.com/codesearch?q=(read|write)\(FILENO_
>
> You mean
http://codesearch.google.com/codesearch?q=(read|write)\(STD(IN|OUT|ERR)_FILENO

Well that's depressing.
Go already lost that battle though.

Russ

[bradfitz, 2011-04-04 20:43:22]

Hello r, r2, rsc1, rsc, jacek.masiulaniec@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-04-04 20:49:01]

LGTM

[bradfitz, 2011-04-04 20:53:54]

*** Submitted as 4d35898bb495 ***

os: add Seek whence constants

R=r, r2, rsc1, rsc, jacek.masiulaniec
CC=golang-dev
http://codereview.appspot.com/4344062