code review 812044: syscall package: document that errno will be zero on success

syscall package: document that errno is zero on success

This is a documentation enhancement only, without any code
change.

The rationale for documenting this precisely is that Unix
programmers who "know" that errno's value is undefined after
a successful system call may be surprised otherwise and
search to be sure that a zero errno may be relied upon after
successful calls.

[giles, 2010-04-01 04:16:14]

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

I'd like you to review this change.

[rsc, 2010-04-01 04:55:10]

This is an odd sentence, because you're not talking
about the underlying system; you're talking about libc,
which is an unrelated sibling system, more or less.
The kernel always signals success or failure; it is libc
that only sometimes copies it into the global errno.
Perhaps instead add the lines

// These calls return errno == 0 to indicate success;
// otherwise errno is an operating system error number
// describing the failure.

?

Russ

[r2, 2010-04-01 04:58:32]

I don't understand.  There is no global errno in package syscall.  Each system
call returns the error code that resulted from its invocation.  I consider this
a feature so transparent to be not worth documenting.

-rob

On Mar 31, 2010, at 9:16 PM, giles.lean@pobox.com wrote:

> Reviewers: r,
> 
> Message:
> Hello r (cc: golang-dev@googlegroups.com),
> 
> I'd like you to review this change.
> 
> 
> Description:
> syscall package: document that errno is zeroed on success
> 
> This is a documentation enhancement only, without any code
> change.
> 
> The rationale for documenting this precisely is that Unix
> programmers who "know" that errno's value is undefined after
> a successful system call may be surprised otherwise and
> search to be sure that a zero errno may be relied upon after
> successful calls.
> 
> Please review this at http://codereview.appspot.com/812044/show
> 
> Affected files:
>  M src/pkg/syscall/syscall.go
> 
> 
> Index: src/pkg/syscall/syscall.go
> ===================================================================
> --- a/src/pkg/syscall/syscall.go
> +++ b/src/pkg/syscall/syscall.go
> @@ -1,4 +1,4 @@
> -// Copyright 2009 The Go Authors. All rights reserved.
> +// Copyright 2009,2010 The Go Authors. All rights reserved.
> // Use of this source code is governed by a BSD-style
> // license that can be found in the LICENSE file.
> 
> @@ -8,7 +8,9 @@
> // interface to the system, such as "os", "time" and "net".  Use those
> // packages rather than this one if you can.
> // For details of the functions and data types in this package consult
> -// the manuals for the appropriate operating system.
> +// the manuals for the appropriate operating system, and note that this
> +// package will zero errno any time a system call returns success
> +// whether or not the underlying system makes such a guarantee.
> package syscall
> 
> func Syscall(trap, a1, a2, a3 uintptr) (r1, r2, err uintptr)
> 
> 

[giles, 2010-04-01 06:15:03]

Russ Cox <rsc@golang.org> wrote:

> This is an odd sentence, ...

Yeah, it felt ugly, too.  And I think you're right about why.

> // These calls return errno == 0 to indicate success;
> // otherwise errno is an operating system error number
> // describing the failure.

I like that a lot better.

On the other hand, Rob doesn't think much of the proposal at
all:

| I don't understand.  There is no global errno in package
| syscall.  Each system call returns the error code that
| resulted from its invocation.  I consider this a feature so
| transparent to be not worth documenting.

With my kernel programmer's hat on, I can now understand that.

With my libc support engineer's hat on, I am willing to state
that it's not going to be transparent to everyone.  Trivial
proof: it wasn't to me, and while in 20:20 hindsight I am
disappointed that I didn't "get it" immediately, I wonder
if my moment of obtunesness will be unique.

I think the one sentence change is worth it to help avoid
future confused people, but Rob if you prefer the description
how it is now, that's how it will be, naturally.

I will submit once more with Russ' wording, and await an
official "OK", or "OK, looks pointless to me but I'll accept
it", or "Abandon the CL, please".

Giles

[giles, 2010-04-01 06:30:17]

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

Please take another look.

[rsc1, 2010-04-06 06:40:13]

LGTM

but remove the year change - we don't
bother updating the copyright year as the
file ages.

[giles, 2010-04-06 16:03:02]

Russ Cox <rsc@google.com> wrote:

> LGTM
> 
> but remove the year change - we don't
> bother updating the copyright year as the
> file ages.

Done.  (Will also attend to any other CLs I have opened or
planned.  It's different to what I'm used to, but I'm sure
Google has lawyers and I'm for sure not one!)

Thanks,

Giles

[giles, 2010-04-06 16:03:13]

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

Please take another look.

[rsc1, 2010-04-06 17:28:56]

LGTM

[rsc, 2010-04-06 17:28:58]

*** Submitted as http://code.google.com/p/go/source/detail?r=0a46c83477db ***

syscall package: document that errno is zeroed on success

This is a documentation enhancement only, without any code
change.

The rationale for documenting this precisely is that Unix
programmers who "know" that errno's value is undefined after
a successful system call may be surprised otherwise and
search to be sure that a zero errno may be relied upon after
successful calls.

R=r, rsc1, rsc
CC=golang-dev
http://codereview.appspot.com/812044

Committer: Russ Cox <rsc@golang.org>

[r2, 2010-04-06 17:31:20]

On Apr 6, 2010, at 10:28 AM, rsc@golang.org wrote:

> *** Submitted as
> http://code.google.com/p/go/source/detail?r=0a46c83477db ***
>
> syscall package: document that errno is zeroed on success

another inaccurate CL description. please let's try to keep the  
descriptions up to date as the review evolves. the descriptions are  
what we see on dashboards, logs, and so on.

-rob