code review 6268045: syscall: use pipe instead of pipe2 on NetBSD

syscall: use pipe instead of pipe2 on NetBSD

pipe2 is equivalent to pipe with flags set to 0.
However, pipe2 was only added recently. Using pipe
instead improves compatibility with NetBSD 5.

[bsiegert, 2012-06-03 14:26:30]

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

I'd like you to review this change to
http://code.google.com/p/go

[bsiegert, 2012-06-03 14:33:20]

I am trying to run go on NetBSD-5.99.51, which I cannot upgrade due to
other reasons, and which does not have pipe2. According to the
documentation, pipe2 is equivalent to pipe with flags=0. syscall.Pipe
always calls with flags=0, so this should be an easy change.

However, when running "go test runtime" after applying this, I get:

warning: building out-of-date packages:
        syscall
        time
        os
        path/filepath
        io/ioutil
        os/exec
        fmt
        flag
        runtime/pprof
        testing
        text/template/parse
        text/template
installing these packages with 'go test -i runtime' will speed future tests.

pipe returned 00 (0x0,0x0)
pipe returned 00 (0x0,0x0)
--- FAIL: TestCrashHandler (1.32 seconds)
        crash_test.go:53: program exited with error: fork/exec
/home/bsiegert/go/bin/go: bad file descriptor
FAIL
FAIL    runtime 4.011s


I am out of ideas at this point. What's more, pipe(2) says that the
signature is

     int
     pipe(int fildes[2]);

     int
     pipe2(int fildes[2], int flags);

While syscalls.master says

42      STD     RUMP    { int|sys||pipe(void); }

Why void? (I guess this is more of a NetBSD question than a Go one.)

--Benny.

[bsiegert, 2012-06-03 15:34:52]

Found it. The signature is fd1, fd2, err = pipe().

[bsiegert, 2012-06-03 15:51:26]

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

Please take another look.

[bsiegert, 2012-06-03 15:52:32]

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

Please take another look.

[jsing, 2012-06-03 17:11:19]

On 2012/06/03 14:33:20, bsiegert wrote:
> I am trying to run go on NetBSD-5.99.51, which I cannot upgrade due to
> other reasons, and which does not have pipe2. According to the
> documentation, pipe2 is equivalent to pipe with flags=0. syscall.Pipe
> always calls with flags=0, so this should be an easy change.
> 
> However, when running "go test runtime" after applying this, I get:
> 
> warning: building out-of-date packages:
>         syscall
>         time
>         os
>         path/filepath
>         io/ioutil
>         os/exec
>         fmt
>         flag
>         runtime/pprof
>         testing
>         text/template/parse
>         text/template
> installing these packages with 'go test -i runtime' will speed future tests.
> 
> pipe returned 00 (0x0,0x0)
> pipe returned 00 (0x0,0x0)
> --- FAIL: TestCrashHandler (1.32 seconds)
>         crash_test.go:53: program exited with error: fork/exec
> /home/bsiegert/go/bin/go: bad file descriptor
> FAIL
> FAIL    runtime 4.011s
> 
> 
> I am out of ideas at this point. What's more, pipe(2) says that the
> signature is
> 
>      int
>      pipe(int fildes[2]);
> 
>      int
>      pipe2(int fildes[2], int flags);
> 
> While syscalls.master says
> 
> 42      STD     RUMP    { int|sys||pipe(void); }
> 
> Why void? (I guess this is more of a NetBSD question than a Go one.)
> 
> --Benny.

As you've discovered, the file descriptors are returned as part of the return
value of the syscall. The libc interface (for example, on i386 the code in
src/lib/libc/i386/sys/pipe.S) converts this into the form that is documented in
the man page.

I think that we probably do not want to officially support Go on NetBSD until
after the 6.0 release - we simply cannot run on 5.1, so we're currently running
on 5.99.x or current. However, having said that I do not see any reason not to
use pipe() instead of pipe2() - it was just convenient since it was semantically
similar to the code that it was cloned from (FWIW pipe2 was added around a year
ago).

Is this the only thing stopping Go from running on your machine?

[bsiegert, 2012-06-03 20:09:28]

> I think that we probably do not want to officially support Go on NetBSD
> until after the 6.0 release - we simply cannot run on 5.1, so we're
> currently running on 5.99.x or current. However, having said that I do
> not see any reason not to use pipe() instead of pipe2() - it was just
> convenient since it was semantically similar to the code that it was
> cloned from (FWIW pipe2 was added around a year ago).

I agree. I still would like to use it on my 5.99.51 though ;).

> Is this the only thing stopping Go from running on your machine?

Yes. With this CL, all.bash runs through and gives the same output
under NetBSD 5.99.51 and 6.0_BETA2.

Both with and without this patch, there are some tests that fail
during all.bash. I created Issue 3697 to track them.

--Benny.

[jsing, 2012-06-04 15:27:51]

On 2012/06/03 20:09:28, bsiegert wrote:
> > I think that we probably do not want to officially support Go on NetBSD
> > until after the 6.0 release - we simply cannot run on 5.1, so we're
> > currently running on 5.99.x or current. However, having said that I do
> > not see any reason not to use pipe() instead of pipe2() - it was just
> > convenient since it was semantically similar to the code that it was
> > cloned from (FWIW pipe2 was added around a year ago).
> 
> I agree. I still would like to use it on my 5.99.51 though ;).

Understood :)

> > Is this the only thing stopping Go from running on your machine?
> 
> Yes. With this CL, all.bash runs through and gives the same output
> under NetBSD 5.99.51 and 6.0_BETA2.

Okay, good.

> Both with and without this patch, there are some tests that fail
> during all.bash. I created Issue 3697 to track them.

Thanks, there are still a couple of known issues. The net/http failure is not
one that I've seen. Do you hit this consistently?

[jsing, 2012-06-04 15:28:08]

LGTM

[jsing, 2012-06-04 15:29:46]

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

syscall: use pipe instead of pipe2 on NetBSD

pipe2 is equivalent to pipe with flags set to 0.
However, pipe2 was only added recently. Using pipe
instead improves compatibility with NetBSD 5.

R=jsing
CC=golang-dev
http://codereview.appspot.com/6268045

Committer: Joel Sing <jsing@google.com>