code review 5128048: net: Set family in newFileFD to fix "unexpected socket ...

net: set family in newFileFD to fix "unexpected socket family" error from WriteToUDP.

[albert.strasheim, 2011-09-26 14:47:23]

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

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

[rsc, 2011-09-26 15:17:48]

Sounds good, but please add a test case.

[albert.strasheim, 2011-09-26 15:50:54]

PTAL.

On 2011/09/26 15:17:48, rsc wrote:
> Sounds good, but please add a test case.

I added a simple WriteTo for the packet conn tests that reproduces the error we
were seeing.

Regards

Albert

[albert.strasheim, 2011-09-30 07:19:53]

ping?

On 2011/09/26 15:50:54, albert.strasheim wrote:
> PTAL.
> 
> On 2011/09/26 15:17:48, rsc wrote:
> > Sounds good, but please add a test case.
> 
> I added a simple WriteTo for the packet conn tests that reproduces the error
we
> were seeing.
> 
> Regards
> 
> Albert

[albert.strasheim, 2011-10-07 08:23:38]

ping?

[mikio, 2011-10-07 08:41:48]

LGTM.
s/net: Set family.../net: set family.../, probably.

http://codereview.appspot.com/5128048/diff/7001/src/pkg/net/file.go
File src/pkg/net/file.go (right):

http://codereview.appspot.com/5128048/diff/7001/src/pkg/net/file.go#newcode25
src/pkg/net/file.go:25: family := 0
s/0/syscall.AF_UNSPEC/

[albert.strasheim, 2011-10-07 09:52:05]

PTAL.

On 2011/10/07 08:41:48, mikioh wrote:
> LGTM.
> s/net: Set family.../net: set family.../, probably.

Fixed.

> http://codereview.appspot.com/5128048/diff/7001/src/pkg/net/file.go
> File src/pkg/net/file.go (right):
> 
> http://codereview.appspot.com/5128048/diff/7001/src/pkg/net/file.go#newcode25
> src/pkg/net/file.go:25: family := 0
> s/0/syscall.AF_UNSPEC/

Fixed.

[mikio, 2011-10-08 05:02:24]

http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go
File src/pkg/net/file_test.go (right):

http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go#new...
src/pkg/net/file_test.go:88: if _, err := c.WriteTo([]byte{}, c.LocalAddr());
err != nil {
On darwin, freebsd c.WriteTo via testFilePacketConnDial fails.

--- FAIL: net.TestFilePacketConn (0.00 seconds)
	WriteTo failed: write udp 127.0.0.1:12345: socket is already connected

[albert.strasheim, 2011-10-08 05:36:59]

Hello

On 2011/10/08 05:02:24, mikioh wrote:
> http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go
> File src/pkg/net/file_test.go (right):
> 
>
http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go#new...
> src/pkg/net/file_test.go:88: if _, err := c.WriteTo([]byte{}, c.LocalAddr());
> err != nil {
> On darwin, freebsd c.WriteTo via testFilePacketConnDial fails.
> 
> --- FAIL: net.TestFilePacketConn (0.00 seconds)
> 	WriteTo failed: write udp 127.0.0.1:12345: socket is already connected

Could you perhaps provide an strace or equivalent? I can't think of any reason
why a UDP socket would return that error.

Regards

Albert

[albert.strasheim, 2011-10-10 08:35:58]

PTAL.

It seems the tests for change 5241043 ran into this same issue.

As far as I understood from looking at those changes, I should only be testing
WriteTo with sockets from ListenPacket, not ones from Dial.

Regards

Albert

On 2011/10/08 05:02:24, mikioh wrote:
> http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go
> File src/pkg/net/file_test.go (right):
> 
>
http://codereview.appspot.com/5128048/diff/17001/src/pkg/net/file_test.go#new...
> src/pkg/net/file_test.go:88: if _, err := c.WriteTo([]byte{}, c.LocalAddr());
> err != nil {
> On darwin, freebsd c.WriteTo via testFilePacketConnDial fails.
> 
> --- FAIL: net.TestFilePacketConn (0.00 seconds)
> 	WriteTo failed: write udp 127.0.0.1:12345: socket is already connected

[rsc, 2011-10-12 17:35:58]

LGTM

[rsc, 2011-10-12 17:36:47]

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

net: fix "unexpected socket family" error from WriteToUDP.

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

Committer: Russ Cox <rsc@golang.org>