code review 4437053: net: use C library resolver on FreeBSD, Linux, OS X / a...

net: use C library resolver on FreeBSD, Linux, OS X / amd64, 386

This CL makes it possible to resolve DNS names on OS X
without offending the Application-Level Firewall.

It also means that cross-compiling from one operating
system to another is no longer possible when using
package net, because cgo needs to be able to sniff around
the local C libraries.  We could special-case this one use
and check in generated files, but it seems more trouble
than it's worth.  Cross compiling is dead anyway.

It is still possible to use either GOARCH=amd64 or GOARCH=386
on typical Linux and OS X x86 systems.

It is also still possible to build GOOS=linux GOARCH=arm on
any system, because arm is for now excluded from this change
(there is no cgo for arm yet).

[rsc, 2011-04-18 23:18:45]

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

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

[rsc, 2011-04-18 23:22:29]

Changed to R=iant.

[r, 2011-04-18 23:27:29]

LGTM but i am not a competent reviewer.
perhaps the package docs for net should mention what's going on?
when one comes to debug the network, it might help to understand the
implementation

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/dnsclient.go
File src/pkg/net/dnsclient.go (right):

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/dnsclient.go#newc...
src/pkg/net/dnsclient.go:362: func goLookupCNAME(name string) (cname string, err
os.Error) {
explain why it's not usually used

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/ipsock.go
File src/pkg/net/ipsock.go (right):

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/ipsock.go#newcode237
src/pkg/net/ipsock.go:237: } else if len(net) >= 4 && net[3] == '6' {
not important but you can do the len check once.

[iant, 2011-04-19 04:55:15]

LGTM

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/cgo_unix.go
File src/pkg/net/cgo_unix.go (right):

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/cgo_unix.go#newco...
src/pkg/net/cgo_unix.go:99: str = C.GoString(C.gai_strerror(gerrno))
If you really want to get it right, then if gerrno == C.EAI_SYSTEM, then the
actual error is in errno.  At least with glibc, gai_strerror(EAI_SYSTEM) returns
"System error".

[mikio, 2011-04-19 05:03:56]

Looks good to me, thanks!

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/cgo_unix.go
File src/pkg/net/cgo_unix.go (right):

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/cgo_unix.go#newco...
src/pkg/net/cgo_unix.go:63: default:
Should we check r.ai_socktype or r.ai_protocol? (same as below)

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/cgo_unix.go#newco...
src/pkg/net/cgo_unix.go:114: switch r.ai_family {
Should we check r.ai_socktype or r.ai_protocol to avoid duplicate addresses
return? In this case getaddrinfo returns both 2011::dead:beaf for tcp and
2011::dead:beef for udp.

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/ipsock.go
File src/pkg/net/ipsock.go (right):

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/ipsock.go#newcode38
src/pkg/net/ipsock.go:38: func firstSupportedAddr(filter func(IP) IP, addrs
[]string) IP {
I'd prefer firstSupportedIP because this guy returns IP object, not IPAddr
object.

http://codereview.appspot.com/4437053/diff/5001/src/pkg/net/ipsock.go#newcode146
src/pkg/net/ipsock.go:146: 
needless linebreak.

[rsc, 2011-04-20 19:21:50]

I made all the suggested changes except
renaming firstSupportedAddr.  LookupHost
returns addrs []string after all.

[rsc, 2011-04-20 19:22:03]

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

net: use C library resolver on FreeBSD, Linux, OS X / amd64, 386

This CL makes it possible to resolve DNS names on OS X
without offending the Application-Level Firewall.

It also means that cross-compiling from one operating
system to another is no longer possible when using
package net, because cgo needs to be able to sniff around
the local C libraries.  We could special-case this one use
and check in generated files, but it seems more trouble
than it's worth.  Cross compiling is dead anyway.

It is still possible to use either GOARCH=amd64 or GOARCH=386
on typical Linux and OS X x86 systems.

It is also still possible to build GOOS=linux GOARCH=arm on
any system, because arm is for now excluded from this change
(there is no cgo for arm yet).

R=iant, r, mikioh
CC=golang-dev
http://codereview.appspot.com/4437053