code review 5162050: exec: add Command.ExtraFiles

exec: add Command.ExtraFiles

Allows passing extra fds to the child process.

Fixes issue 2329

[bradfitz, 2011-10-03 21:49:31]

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

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

[rsc, 2011-10-03 21:52:20]

I am not sure.
Not everything supports 'extra files'.
The os.ProcAttr comment is very clear
that only the first 3 are guaranteed to work.
At the least the test will fail on Windows.

Also ExtraFiles seems kind of clumsy.

Russ

[dsymonds, 2011-10-03 21:52:59]

LGTM

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go
File src/pkg/exec/exec.go (right):

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go#newcode70
src/pkg/exec/exec.go:70: ExtraFiles []*os.File
What about just calling it "Files"?

[bradfitz, 2011-10-03 21:54:05]

I saw some code that had to use os.StartProcess instead of exec because of
lack of this support.  It was more clumsy than this.

Why don't we just return an os.Error if the platform doesn't support it?


On Mon, Oct 3, 2011 at 2:52 PM, Russ Cox <rsc@golang.org> wrote:

> I am not sure.
> Not everything supports 'extra files'.
> The os.ProcAttr comment is very clear
> that only the first 3 are guaranteed to work.
> At the least the test will fail on Windows.
>
> Also ExtraFiles seems kind of clumsy.
>
> Russ
>

[bradfitz, 2011-10-03 21:54:25]

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go
File src/pkg/exec/exec.go (right):

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go#newcode70
src/pkg/exec/exec.go:70: ExtraFiles []*os.File
On 2011/10/03 21:52:59, dsymonds wrote:
> What about just calling it "Files"?

Then it would be named the same but behave differently from the os one.

[rsc, 2011-10-03 21:57:11]

I know we have to do something.
Let me mull it overnight.

Russ

[rsc, 2011-10-06 17:50:58]

LGTM

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go
File src/pkg/exec/exec.go (right):

http://codereview.appspot.com/5162050/diff/5001/src/pkg/exec/exec.go#newcode68
src/pkg/exec/exec.go:68: // standard error. If non-nil, the first entry in
ExtraFiles will become
// If non-nil, entry i becomes file descriptor 3+i.

[bradfitz, 2011-10-06 18:00:12]

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

exec: add Command.ExtraFiles

Allows passing extra fds to the child process.

Fixes issue 2329

R=rsc, dsymonds
CC=golang-dev
http://codereview.appspot.com/5162050