code review 11932044: net: make UnixAddr implement sockaddr interface

net: make UnixAddr implement sockaddr interface

This is in preparation for runtime-integrated network pollster for BSD
variants.

Update issue 5199

[mikio, 2013-07-28 04:05:00]

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

[dave_cheney.net, 2013-07-28 04:08:28]

Very close, thank you for breaking this down.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix.go
File src/pkg/net/unixsock_posix.go (right):

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:106: func (a *UnixAddr) isWildcard() bool {
please add a small comment, isUnnamed didn't really explain what it did, neither
does isWildcard.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:107: if a == nil || a.Name == "" {
can this be written as 

return a == nil || a.Name == ""

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:120: func (a *UnixAddr) toAddr() sockaddr {
can this just be written as return a ? or will that introduce a typed nil ?

[mikio, 2013-07-28 06:12:26]

ptal

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix.go
File src/pkg/net/unixsock_posix.go (right):

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:106: func (a *UnixAddr) isWildcard() bool {
On 2013/07/28 04:08:28, dfc wrote:
> please add a small comment, isUnnamed didn't really explain what it did,
neither
> does isWildcard.

Done.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:107: if a == nil || a.Name == "" {
On 2013/07/28 04:08:28, dfc wrote:
> can this be written as 
> 
> return a == nil || a.Name == ""

Done.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:107: if a == nil || a.Name == "" {
On 2013/07/28 04:08:28, dfc wrote:
> can this be written as 
> 
> return a == nil || a.Name == ""

Done.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:120: func (a *UnixAddr) toAddr() sockaddr {
Sorry, but I don't get it.
Well, are you suggesting to change the behavior on ReadFrom of PacketConn?

[dave_cheney.net, 2013-07-28 06:18:58]

LGTM. I don't know if you want to have another pass to address the remaining
comments, or tackle them in a later CL.

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix.go
File src/pkg/net/unixsock_posix.go (right):

https://codereview.appspot.com/11932044/diff/6001/src/pkg/net/unixsock_posix....
src/pkg/net/unixsock_posix.go:120: func (a *UnixAddr) toAddr() sockaddr {
On 2013/07/28 06:12:27, mikio wrote:
> Sorry, but I don't get it.
> Well, are you suggesting to change the behavior on ReadFrom of PacketConn?

I think leave it for this change, but could this method be simplified to be 

func (a *UnixAddr) toAddr() sockaddr { return a }

https://codereview.appspot.com/11932044/diff/11001/src/pkg/net/unixsock_posix.go
File src/pkg/net/unixsock_posix.go (right):

https://codereview.appspot.com/11932044/diff/11001/src/pkg/net/unixsock_posix...
src/pkg/net/unixsock_posix.go:107: // It's dfferent between platforms, but some
platforms can
I'd suggest a less conversational tone, perhaps (and put the comment on the
method, not the body)

// isWildcard indicates if this *UnixAddr represents a wildcard address. 
// This behaviour is platform dependent.

Or, avoid renaming the method from isUnamed.

[mikio, 2013-07-28 07:11:08]

On Sun, Jul 28, 2013 at 3:18 PM,  <dave@cheney.net> wrote:

>> Sorry, but I don't get it.
>> Well, are you suggesting to change the behavior on ReadFrom of PacketConn?
>
> I think leave it for this change, but could this method be simplified to be
>
> func (a *UnixAddr) toAddr() sockaddr { return a }

Then we'll see some errors. ;)

% go test
--- FAIL: TestReadUnixgramWithUnnamedSocket-2 (0.00 seconds)
unix_test.go:59: neighbor address is <nil>
--- FAIL: TestReadUnixgramWithZeroBytesBuffer-2 (0.00 seconds)
unix_test.go:106: neighbor address is <nil>
FAIL

[mikio, 2013-07-28 07:12:07]

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

Please take another look.

[dave_cheney.net, 2013-07-28 07:12:26]

Ahh, ok, I understand the leak typed nils, please ignore my suggestion.

On Sun, Jul 28, 2013 at 5:11 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> On Sun, Jul 28, 2013 at 3:18 PM,  <dave@cheney.net> wrote:
>
>>> Sorry, but I don't get it.
>>> Well, are you suggesting to change the behavior on ReadFrom of PacketConn?
>>
>> I think leave it for this change, but could this method be simplified to be
>>
>> func (a *UnixAddr) toAddr() sockaddr { return a }
>
> Then we'll see some errors. ;)
>
> % go test
> --- FAIL: TestReadUnixgramWithUnnamedSocket-2 (0.00 seconds)
> unix_test.go:59: neighbor address is <nil>
> --- FAIL: TestReadUnixgramWithZeroBytesBuffer-2 (0.00 seconds)
> unix_test.go:106: neighbor address is <nil>
> FAIL

[dave_cheney.net, 2013-07-28 07:13:14]

Thanks. LGTM.

[mikio, 2013-07-28 07:15:15]

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

net: make UnixAddr implement sockaddr interface

This is in preparation for runtime-integrated network pollster for BSD
variants.

Update issue 5199

R=golang-dev, dave
CC=golang-dev
https://codereview.appspot.com/11932044