code review 4244055: net: drop laddr from Dial, cname from LookupHost; new f...

net: drop laddr from Dial, cname from LookupHost; new functions

Drop laddr argument from Dial.

Drop cname return from LookupHost.

Add LookupIP, LookupCNAME, ParseCIDR, IP.Equal.
Export SplitHostPort, JoinHostPort.
Add AAAA (IPv6) support to host lookups.

Preparations for implementing some of the
lookups using cgo.

ParseCIDR and IP.Equal are logically new in this CL
but accidentally snuck into an earlier CL about unused
labels that was in the same client.

In crypto/tls, drop laddr from Dial to match net.

[rsc, 2011-03-16 16:14:53]

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

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

[dsymonds, 2011-03-16 16:22:05]

http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/dial.go
File src/pkg/net/dial.go (left):

http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/dial.go#oldcode35
src/pkg/net/dial.go:35: if raddr != "" {
shouldn't raddr always be non-empty? There's no use calling Dial("tcp", "") is
there? You could lose this if block and let the inner one raise an error.

Same for the other protocols.

http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/lookup.go
File src/pkg/net/lookup.go (right):

http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/lookup.go#newcode11
src/pkg/net/lookup.go:11: // LookupHost looks up the given host using the local
resolver.
"Lookup" as a verb has many opponents. I don't feel strongly about it, but it is
a little weak here. What about naming all these Resolve*?

[rsc, 2011-03-16 16:24:49]

>
http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/lookup.go#newcode11
> src/pkg/net/lookup.go:11: // LookupHost looks up the given host using
> the local resolver.
> "Lookup" as a verb has many opponents. I don't feel strongly about it,
> but it is a little weak here. What about naming all these Resolve*?

Sorry, they've been Lookup from day 1.

Russ

[rsc, 2011-03-16 16:28:37]

> http://codereview.appspot.com/4244055/diff/15001/src/pkg/net/dial.go#oldcode35
> src/pkg/net/dial.go:35: if raddr != "" {
> shouldn't raddr always be non-empty? There's no use calling Dial("tcp",
> "") is there? You could lose this if block and let the inner one raise
> an error.

Done.

Russ

[adg, 2011-03-17 01:06:39]

LGTM

Perhaps mention that SyntaxError became ParseError in the change notes.

[rh, 2011-03-17 04:17:23]

Nice.

http://codereview.appspot.com/4244055/diff/21001/src/pkg/net/ip_test.go
File src/pkg/net/ip_test.go (right):

http://codereview.appspot.com/4244055/diff/21001/src/pkg/net/ip_test.go#newco...
src/pkg/net/ip_test.go:135: }
Worth testing bad values to SplitHostPort return an error?

[rsc, 2011-03-17 05:08:01]

On Wed, Mar 16, 2011 at 21:06,  <adg@golang.org> wrote:
> LGTM
>
> Perhaps mention that SyntaxError became ParseError in the change notes.

Nah.  The fact that that code is checked in at all was
a bug (as mentioned in the CL) and not something we
ever advertised.

Russ

[rsc, 2011-03-29 03:28:45]

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

net: drop laddr from Dial, cname from LookupHost; new functions

Drop laddr argument from Dial.

Drop cname return from LookupHost.

Add LookupIP, LookupCNAME, ParseCIDR, IP.Equal.
Export SplitHostPort, JoinHostPort.
Add AAAA (IPv6) support to host lookups.

Preparations for implementing some of the
lookups using cgo.

ParseCIDR and IP.Equal are logically new in this CL
but accidentally snuck into an earlier CL about unused
labels that was in the same client.

In crypto/tls, drop laddr from Dial to match net.

R=golang-dev, dsymonds, adg, rh
CC=golang-dev
http://codereview.appspot.com/4244055