code review 6063053: syscall: add Termios support on darwin

syscall: add Termios support on darwin

Update issue 3316.

[fss, 2012-04-19 17:17:11]

Hello golang-dev@googlegroups.com,

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

[fss, 2012-04-23 03:18:37]

Ping?

[dfc, 2012-04-23 08:56:58]

On 2012/04/23 03:18:37, fsouza wrote:
> Ping?

I'm not sure what the policy is currently on adding new exported symbols to
pkg/syscall. What were you planning on doing with these ?

[kevlar, 2012-04-23 20:19:35]

FWIW, these are relatively straightforward to do directly in terminal packages. 
I'd only put them in the stdlib if they were used in the stdlib or were
relatively easy to use directly (which I'd argue is not the case here).

https://github.com/kylelemons/goat/blob/go.r60/term/termio.go

[fss, 2012-04-24 13:25:53]

On 2012/04/23 20:19:35, kevlar wrote:
> FWIW, these are relatively straightforward to do directly in terminal
packages. 
> I'd only put them in the stdlib if they were used in the stdlib or were
> relatively easy to use directly (which I'd argue is not the case here).
> 
> https://github.com/kylelemons/goat/blob/go.r60/term/termio.go

I agree, but what happens to termios support on linux? It is in the stdlib

[dho, 2012-04-24 14:18:53]

Op 23 april 2012 04:56 heeft  <dave@cheney.net> het volgende geschreven:
> On 2012/04/23 03:18:37, fsouza wrote:
>>
>> Ping?
>
>
> I'm not sure what the policy is currently on adding new exported symbols
> to pkg/syscall. What were you planning on doing with these ?

There are only two marginally reasonable reasons I think package
syscall is incomplete:

1) The tendency of Go to encourage portable programs
2) We only pulled in the ones we needed for OS-specific code in the
runtime and standard library

Many syscalls aren't frequently used and because of #2, it was more of
a time waste to attempt to implement them all than it was worth.

I don't think that syscall being an incomplete package because we
didn't need all of them for the standard library is a good argument.
It's a pain that there's a non-portable syscall interface (that's made
even more non-portable by using the sub-interface of ioctls) that has
marginally similar implementations across at least 3 operating systems
we have first tier support for, and I can understand the consistency
argument. Termios is defined for Linux.

I don't think we should be against adding things to package syscall
just because people can write non-portable programs with them. The
syscall package docs even admit this a bit with "Use [the standard
library] packages rather than this one *if you can.*" (emphasis mine).
Anybody needing to use syscalls is likely to understand these
implications and be ok with that. If Go doesn't wish to let people use
syscalls, then syscall.RawSyscall shouldn't be exported. Otherwise, I
don't see any reason to deny a CL making a systems interface easier to
use for a tier 1 target, especially if someone already provided the
code.

From a code perspective, LGTM.

--dho

[rsc, 2012-04-25 01:42:22]

See the note at the top of the z* files.

http://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darwi...
File src/pkg/syscall/zerrors_darwin_386.go (left):

http://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darwi...
src/pkg/syscall/zerrors_darwin_386.go:149: ECHO                              =
0x8
Why did this change?  These are auto-generated files but I don't see anything
changing in the generators.  Also, usually we handle constants by adding them
here (via changes in the generators) not by explicit enumeration in
types_darwin.go.

[fss, 2012-04-26 23:59:41]

On 2012/04/25 01:42:22, rsc wrote:
> See the note at the top of the z* files.
> 
>
http://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darwi...
> File src/pkg/syscall/zerrors_darwin_386.go (left):
> 
>
http://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darwi...
> src/pkg/syscall/zerrors_darwin_386.go:149: ECHO                              =
> 0x8
> Why did this change?  These are auto-generated files but I don't see anything
> changing in the generators.  Also, usually we handle constants by adding them
> here (via changes in the generators) not by explicit enumeration in
> types_darwin.go.

ouch, sorry about that. Actually, I did not use the go generator (I didn't read
the file headers, sorry again about that) and ended up creating some script to
generate these constants.

I'll take a look in the generators and get back here.

[fss, 2012-04-27 04:52:04]

Please take another look.

I've changed mkerrors.sh to include termios.h constants in go files, and
regenerates zerrors_darwin_* using mkall.sh.

https://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darw...
File src/pkg/syscall/zerrors_darwin_386.go (left):

https://codereview.appspot.com/6063053/diff/5001/src/pkg/syscall/zerrors_darw...
src/pkg/syscall/zerrors_darwin_386.go:149: ECHO                              =
0x8
On 2012/04/25 01:42:22, rsc wrote:
> Why did this change?  These are auto-generated files but I don't see anything
> changing in the generators.  Also, usually we handle constants by adding them
> here (via changes in the generators) not by explicit enumeration in
> types_darwin.go.

Done.

[minux1, 2012-04-27 04:58:01]

https://codereview.appspot.com/6063053/diff/18001/src/pkg/syscall/mkerrors.sh
File src/pkg/syscall/mkerrors.sh (right):

https://codereview.appspot.com/6063053/diff/18001/src/pkg/syscall/mkerrors.sh...
src/pkg/syscall/mkerrors.sh:178: $2 ~ /^V[A-Z0-9]+$/ ||
please use tab to indent for consistency.

[fss, 2012-04-27 05:30:01]

ptal

https://codereview.appspot.com/6063053/diff/18001/src/pkg/syscall/mkerrors.sh
File src/pkg/syscall/mkerrors.sh (right):

https://codereview.appspot.com/6063053/diff/18001/src/pkg/syscall/mkerrors.sh...
src/pkg/syscall/mkerrors.sh:178: $2 ~ /^V[A-Z0-9]+$/ ||
On 2012/04/27 04:58:01, minux wrote:
> please use tab to indent for consistency.

Done.

[fss, 2012-05-01 04:27:53]

ping?

[rsc, 2012-05-01 23:05:36]

LGTM

Will submit later if no one beats me to it.

[rsc, 2012-05-03 21:33:23]

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

syscall: add Termios support on darwin

Update issue 3316.

R=dave, kevlar, devon.odell, rsc, minux.ma
CC=golang-dev
http://codereview.appspot.com/6063053

Committer: Russ Cox <rsc@golang.org>