Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(309)

Issue 11958043: code review 11958043: net: make IP address selection work correctly on IPv6-o... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by mikio
Modified:
10 years, 8 months ago
Reviewers:
dfc, bradfitz
CC:
golang-dev, bradfitz, dfc, fvb
Visibility:
Public.

Description

net: make IP address selection work correctly on IPv6-only kernel Update issue 3610 Update issue 5267 Update issue 5707

Patch Set 1 #

Patch Set 2 : diff -r ac410fc1fe6c https://code.google.com/p/go #

Patch Set 3 : diff -r ac410fc1fe6c https://code.google.com/p/go #

Total comments: 7

Patch Set 4 : diff -r cfaef1dc8164 https://code.google.com/p/go #

Total comments: 6

Patch Set 5 : diff -r be6c9de7b802 https://code.google.com/p/go #

Total comments: 10

Patch Set 6 : diff -r 8b61a4ec6d1e https://code.google.com/p/go #

Total comments: 4

Patch Set 7 : diff -r 19591dcb81c6 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -13 lines) Patch
M src/pkg/net/ipsock.go View 1 2 3 4 5 6 2 chunks +36 lines, -13 lines 0 comments Download
M src/pkg/net/ipsock_plan9.go View 1 2 3 4 5 1 chunk +8 lines, -0 lines 0 comments Download
M src/pkg/net/ipsock_posix.go View 1 2 3 4 5 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 13
mikio
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
10 years, 8 months ago (2013-07-27 07:48:27 UTC) #1
bradfitz
Can you update the CL description with an explanation of what this is doing? What ...
10 years, 8 months ago (2013-07-27 16:21:17 UTC) #2
dfc
Thanks for this proposal, here are some comments. I agree with Brad that the description ...
10 years, 8 months ago (2013-07-28 00:37:44 UTC) #3
mikio
Hello golang-dev@googlegroups.com, bradfitz@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 8 months ago (2013-07-28 04:49:43 UTC) #4
mikio
thanks both for the comments, ptal. https://codereview.appspot.com/11958043/diff/6001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/6001/src/pkg/net/ipsock.go#newcode10 src/pkg/net/ipsock.go:10: On 2013/07/28 00:37:44, ...
10 years, 8 months ago (2013-07-28 04:51:08 UTC) #5
dfc
LGTM. Please wait for bradfitz.
10 years, 8 months ago (2013-07-28 04:59:28 UTC) #6
fvb
https://codereview.appspot.com/11958043/diff/13001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/13001/src/pkg/net/ipsock.go#newcode16 src/pkg/net/ipsock.go:16: // supportsIPv6 reports whether the plaffrom supports IPv6 s/plaffrom/platform/ ...
10 years, 8 months ago (2013-07-28 07:35:41 UTC) #7
mikio
Thanks. https://codereview.appspot.com/11958043/diff/13001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/13001/src/pkg/net/ipsock.go#newcode16 src/pkg/net/ipsock.go:16: // supportsIPv6 reports whether the plaffrom supports IPv6 ...
10 years, 8 months ago (2013-07-28 07:41:28 UTC) #8
mikio
ping
10 years, 8 months ago (2013-07-31 03:32:32 UTC) #9
bradfitz
https://codereview.appspot.com/11958043/diff/21001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/21001/src/pkg/net/ipsock.go#newcode20 src/pkg/net/ipsock.go:20: // supportsIPv4map reports whether the platform supports this comment ...
10 years, 8 months ago (2013-08-01 21:44:12 UTC) #10
mikio
PTAL https://codereview.appspot.com/11958043/diff/21001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/21001/src/pkg/net/ipsock.go#newcode20 src/pkg/net/ipsock.go:20: // supportsIPv4map reports whether the platform supports On ...
10 years, 8 months ago (2013-08-02 08:46:46 UTC) #11
bradfitz
LGTM but see comments on comments https://codereview.appspot.com/11958043/diff/30001/src/pkg/net/ipsock.go File src/pkg/net/ipsock.go (right): https://codereview.appspot.com/11958043/diff/30001/src/pkg/net/ipsock.go#newcode25 src/pkg/net/ipsock.go:25: // See RFC ...
10 years, 8 months ago (2013-08-02 17:32:40 UTC) #12
mikio
10 years, 8 months ago (2013-08-03 03:17:13 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=7c06f390d1ad ***

net: make IP address selection work correctly on IPv6-only kernel

Update issue 3610
Update issue 5267
Update issue 5707

R=golang-dev, bradfitz, dave, fvbommel
CC=golang-dev
https://codereview.appspot.com/11958043

https://codereview.appspot.com/11958043/diff/30001/src/pkg/net/ipsock.go
File src/pkg/net/ipsock.go (right):

https://codereview.appspot.com/11958043/diff/30001/src/pkg/net/ipsock.go#newc...
src/pkg/net/ipsock.go:25: // See RFC 4291, RFC 4038 and RFC 3493 for more
inforamtion
On 2013/08/02 17:32:40, bradfitz wrote:
> Actually the whole comment is long.  You can probably just say:
> 
> // supportsIPv4map reports whether the platform supports
> // mapping an IPv4 address inside an IPv6 address.
> // See RFC 4291, 4038 and 3493.
> 
> At least, those sound the same to me, but one has fewer words.

Thanks, will go w/ yours + "mapping ... at transport layer protocols".

https://codereview.appspot.com/11958043/diff/30001/src/pkg/net/ipsock.go#newc...
src/pkg/net/ipsock.go:64: // kernel configuraiton.
On 2013/08/02 17:32:40, bradfitz wrote:
> typo: configuration
> 
> But this comment doesn't say much.  It just says "anyaddr returns a thing". 
I'd
> say something about whether this ever returns nil.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b