code review 5372080: syscall: use error

syscall: use error

- syscall (not os) now defines the Errno type.
- the low-level assembly functions Syscall, Syscall6, and so on
  return Errno, not uintptr
- syscall wrappers all return error, not uintptr.

[rsc, 2011-11-13 23:51:08]

Hello golang-dev@googlegroups.com,

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

[mikio, 2011-11-14 01:12:52]

A nit.


diff -r c3ae7b7f6930 src/pkg/net/http/serve_test.go
--- a/src/pkg/net/http/serve_test.go	Sun Nov 13 14:48:22 2011 -0500
+++ b/src/pkg/net/http/serve_test.go	Mon Nov 14 10:11:35 2011 +0900
@@ -1145,7 +1145,7 @@
 	ln := &errorListener{[]error{
 		&net.OpError{
 			Op:  "accept",
-			Err: os.Errno(syscall.EMFILE),
+			Err: syscall.EMFILE,
 		}}}
 	err := Serve(ln, HandlerFunc(HandlerFunc(func(ResponseWriter, *Request) {})))

[mikio, 2011-11-14 02:23:07]

On 2011/11/14 01:12:52, mikioh wrote:

> A nit.

Ah, CL 5372081 already has the fix.

[r, 2011-11-14 02:32:47]

LGTM
i like what this does for portability; that was unexpected.

not sure if it's relevant but
http://support.ipswitch.com/kb/WSK-19980714-EM29.htm

http://codereview.appspot.com/5372080/diff/3/src/pkg/net/http/transport_windo...
File src/pkg/net/http/transport_windows.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/net/http/transport_windo...
src/pkg/net/http/transport_windows.go:15: if ok && op.Op == "WSARecv" && op.Net
== "tcp" && op.Err == syscall.Errno(10058) {
while we're here, what the hell is 10058?
that TODO could use addressing.

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/mkerrors.sh
File src/pkg/syscall/mkerrors.sh (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/mkerrors.sh#newc...
src/pkg/syscall/mkerrors.sh:205: cat _error.out | grep -f _error.grep | sed
's/=\(.*\)/= Errno(\1)/'
sed with backslashes makes me nervous about portability but i think it's ok
outside plan 9

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/syscall_unix.go
File src/pkg/syscall/syscall_unix.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/syscall_unix.go#...
src/pkg/syscall/syscall_unix.go:87: // An Errno is a positive number describing
an error condition.
well, there's zero too. maybe unsigned number?

[brainman, 2011-11-14 02:44:01]

On 2011/11/14 02:32:47, r wrote:
> 
> ...
> src/pkg/net/http/transport_windows.go:15: if ok && op.Op == "WSARecv" &&
op.Net
> == "tcp" && op.Err == syscall.Errno(10058) {
> while we're here, what the hell is 10058?

I have CL to give it name (http://codereview.appspot.com/5312048/). But, as Russ
said (see CL thread), this Windows specific code shouldn't be here in the first
place. Unfortunately, I have no alternative proposal yet.

Alex

[mikio, 2011-11-14 03:24:38]

LGTM

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_freebsd_...
File src/pkg/syscall/zerrors_freebsd_386.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_freebsd_...
src/pkg/syscall/zerrors_freebsd_386.go:1219: ECHO            = Errno(0x8)
A series of ECHO, ECHO to ECHOPRT, is not included in errno.h.

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_freebsd_...
File src/pkg/syscall/zerrors_freebsd_amd64.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_freebsd_...
src/pkg/syscall/zerrors_freebsd_amd64.go:1219: ECHO            = Errno(0x8)
same as freebsd/386.

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_openbsd_...
File src/pkg/syscall/zerrors_openbsd_386.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_openbsd_...
src/pkg/syscall/zerrors_openbsd_386.go:1185: ECHO            = Errno(0x8)
same as freebsd.

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_openbsd_...
File src/pkg/syscall/zerrors_openbsd_amd64.go (right):

http://codereview.appspot.com/5372080/diff/3/src/pkg/syscall/zerrors_openbsd_...
src/pkg/syscall/zerrors_openbsd_amd64.go:1185: ECHO            = Errno(0x8)
same as freebsd.

[rsc, 2011-11-14 03:27:24]

Thanks.

I edited the z-files for freebsd, openbsd, and windows by hand,
because I do not have the right machines on which to run mkerrors.sh.
I hope that once this is checked in, you or someone else with access
to the appropriate machines can rerun mkall.sh and send a CL with
truly machine-generated z-files.  If ECHO still ends up in the list,
we should fix mkall.sh to exclude it.

Russ

[rsc, 2011-11-14 03:30:35]

On Sun, Nov 13, 2011 at 21:32,  <r@golang.org> wrote:
> src/pkg/syscall/syscall_unix.go:87: // An Errno is a positive number
> describing an error condition.
> well, there's zero too. maybe unsigned number?

now

// An Errno is an unsigned number describing an error condition.
// It implements the error interface.  The zero Errno is by convention
// a non-error, so code to convert from Errno to error should use:
//	err = nil
//	if errno != 0 {
//		err = errno
//	}
type Errno uintptr

[rsc, 2011-11-14 03:44:57]

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

syscall: use error

- syscall (not os) now defines the Errno type.
- the low-level assembly functions Syscall, Syscall6, and so on
  return Errno, not uintptr
- syscall wrappers all return error, not uintptr.

R=golang-dev, mikioh.mikioh, r, alex.brainman
CC=golang-dev
http://codereview.appspot.com/5372080