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

Issue 6525048: code review 6525048: net: protocol specific listen functions return a proper... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 2 months ago by mikio
Modified:
9 years ago
Reviewers:
CC:
rsc, iant, golang-dev
Visibility:
Public.

Description

net: protocol specific listen functions return a proper local socket address When a nil listener address is passed to some protocol specific listen function, it will create an unnamed, unbound socket because of the nil listener address. Other listener functions may return invalid address error. This CL allows to pass a nil listener address to all protocol specific listen functions to fix above inconsistency. Also make it possible to return a proper local socket address in case of a nil listner address. Fixes issue 4190. Fixes issue 3847.

Patch Set 1 : diff -r 284f6a1e531b https://go.googlecode.com/hg/ #

Total comments: 3

Patch Set 2 : diff -r ad0d92901061 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r b661b713984f https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r b4a8d6b52a2f https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r 90e59cec3396 https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 422c00e8e022 https://go.googlecode.com/hg/ #

Total comments: 4

Patch Set 7 : diff -r 0b46d6fe2f1c https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+130 lines, -28 lines) Patch
M src/pkg/net/ipraw_test.go View 1 2 3 4 5 1 chunk +30 lines, -0 lines 0 comments Download
M src/pkg/net/iprawsock_posix.go View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/net/tcp_test.go View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
M src/pkg/net/tcpsock_plan9.go View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/tcpsock_posix.go View 1 2 3 4 3 chunks +35 lines, -23 lines 0 comments Download
M src/pkg/net/udp_test.go View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
M src/pkg/net/udpsock_plan9.go View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/net/udpsock_posix.go View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 13
mikio
Hello rsc@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
9 years, 1 month ago (2012-10-04 07:24:56 UTC) #1
mikio
ping
9 years, 1 month ago (2012-10-13 17:14:57 UTC) #2
iant
Why is it correct to change the existing behaviour of net.ListenIP? https://codereview.appspot.com/6525048/diff/10001/src/pkg/net/tcpsock_posix.go File src/pkg/net/tcpsock_posix.go (right): ...
9 years, 1 month ago (2012-10-13 17:40:49 UTC) #3
rsc
Like Ian, I don't understand why the restrictions are being added. This program succeeds today: ...
9 years, 1 month ago (2012-10-21 21:22:25 UTC) #4
mikio
Sorry for the late response. On 2012/10/13 17:40:49, iant wrote: > Why is it correct ...
9 years ago (2012-11-06 08:23:50 UTC) #5
rsc
Since it seems like half the things already accept nil, let's say that everything should. ...
9 years ago (2012-11-06 18:54:34 UTC) #6
mikio
On 2012/11/06 18:54:34, rsc wrote: > Since it seems like half the things already accept ...
9 years ago (2012-11-07 18:41:15 UTC) #7
mikio
Hello rsc@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years ago (2012-11-07 18:42:15 UTC) #8
mikio
Hello rsc@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years ago (2012-11-08 22:24:25 UTC) #9
mikio
Hello rsc@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years ago (2012-11-10 05:58:56 UTC) #10
mikio
Hello rsc@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
9 years ago (2012-11-12 00:46:50 UTC) #11
rsc
LGTM Glad to see this all uniform. Thanks. https://codereview.appspot.com/6525048/diff/26001/src/pkg/net/tcp_test.go File src/pkg/net/tcp_test.go (right): https://codereview.appspot.com/6525048/diff/26001/src/pkg/net/tcp_test.go#newcode143 src/pkg/net/tcp_test.go:143: if ...
9 years ago (2012-11-12 21:07:46 UTC) #12
mikio
9 years ago (2012-11-13 03:56:50 UTC) #13
*** Submitted as http://code.google.com/p/go/source/detail?r=9b31d3f52fff ***

net: protocol specific listen functions return a proper local socket address

When a nil listener address is passed to some protocol specific
listen function, it will create an unnamed, unbound socket because
of the nil listener address. Other listener functions may return
invalid address error.

This CL allows to pass a nil listener address to all protocol
specific listen functions to fix above inconsistency. Also make it
possible to return a proper local socket address in case of a nil
listner address.

Fixes issue 4190.
Fixes issue 3847.

R=rsc, iant
CC=golang-dev
http://codereview.appspot.com/6525048

http://codereview.appspot.com/6525048/diff/26001/src/pkg/net/tcp_test.go
File src/pkg/net/tcp_test.go (right):

http://codereview.appspot.com/6525048/diff/26001/src/pkg/net/tcp_test.go#newc...
src/pkg/net/tcp_test.go:143: if la == nil {
On 2012/11/12 21:07:46, rsc wrote:
> the ,ok below will handle nil correctly so you could delete this and use a
> instead of la in the print below.

Done.

http://codereview.appspot.com/6525048/diff/26001/src/pkg/net/udp_test.go
File src/pkg/net/udp_test.go (right):

http://codereview.appspot.com/6525048/diff/26001/src/pkg/net/udp_test.go#newc...
src/pkg/net/udp_test.go:114: if la == nil {
On 2012/11/12 21:07:46, rsc wrote:
> same

Done.
Sign in to reply to this message.

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