code review 69910043: syscall: make address of unnamed Unix sockets empty on ...

syscall: make address of unnamed Unix sockets empty on Linux. Fixes issue 7354.

[remyoudompheng, 2014-02-28 07:12:09]

Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/

[bradfitz, 2014-02-28 15:47:04]

LGTM but wait a bit to see if others have feedback

[rsc, 2014-03-04 00:05:17]

Why is the len being introduced?

[remyoudompheng, 2014-03-04 00:15:30]

2014-03-04 1:05 GMT+01:00 <rsc@golang.org>: > Why is the len being introduced? An unnamed socket and an "abstract domain" socket whose address is "\0" will both have a RawSockaddrUnix filled with zeroes, so anyToSockaddr cannot distringuish them without additional information. For the unnamed socket I expect address "" and len=unsafe.Sizeof(rsa.Addr.Family) For the abstract one address "@", and len=unsafe.Sizeof(RawSockaddrUnix). [as mentioned on man unix(7) on Linux]. Rémy. 

[rsc, 2014-03-04 00:48:07]

I think you can drop the new len parameter. We know the length. If nothing else it is in the Sockaddr. https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... File src/pkg/syscall/syscall_linux.go (right): https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... src/pkg/syscall/syscall_linux.go:355: if uintptr(rsalen) == unsafe.Sizeof(rsa.Addr.Family) { I still don't understand why we care about rsalen here. The code building the path below doesn't care. if pp.Path[0] == 0 && pp.Path[1] == 0 { // actual unnamed socket return sa, nil }

[remyoudompheng, 2014-03-07 06:10:07]

https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... File src/pkg/syscall/syscall_linux.go (right): https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... src/pkg/syscall/syscall_linux.go:355: if uintptr(rsalen) == unsafe.Sizeof(rsa.Addr.Family) { On 2014/03/04 00:48:07, rsc wrote: > I still don't understand why we care about rsalen here. The code building the > path below doesn't care. > > if pp.Path[0] == 0 && pp.Path[1] == 0 { // actual unnamed socket > return sa, nil > } > You could have an abstract socket with address just "@", which would then be indistinguishable. But I'm also fine with the "pp.Path[0] == 0 && pp.Path[1] == 0", which was the thing I initially planned.

[mikio, 2014-03-07 07:59:03]

On Fri, Mar 7, 2014 at 3:10 PM, <remyoudompheng@gmail.com> wrote: > You could have an abstract socket with address just "@", which would > then be indistinguishable. But I'm also fine with the "pp.Path[0] == 0 > && pp.Path[1] == 0", which was the thing I initially planned. Once we get a consensus, please add a side note that describes how we handle AF_LOCAL (formerly AF_UNIX) namespace on net.UnixAddr.Name. Thanks. 

[rsc, 2014-04-03 16:10:04]

On 2014/03/07 06:10:07, remyoudompheng wrote: > https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... > File src/pkg/syscall/syscall_linux.go (right): > > https://codereview.appspot.com/69910043/diff/40001/src/pkg/syscall/syscall_li... > src/pkg/syscall/syscall_linux.go:355: if uintptr(rsalen) == > unsafe.Sizeof(rsa.Addr.Family) { > On 2014/03/04 00:48:07, rsc wrote: > > I still don't understand why we care about rsalen here. The code building the > > path below doesn't care. > > > > if pp.Path[0] == 0 && pp.Path[1] == 0 { // actual unnamed socket > > return sa, nil > > } > > > > You could have an abstract socket with address just "@", which would then be > indistinguishable. But I'm also fine with the "pp.Path[0] == 0 && pp.Path[1] == > 0", which was the thing I initially planned. Sorry, I missed this reply somehow. You cannot have an abstract socket with address just '@'. The @ is only ever used during printing. It does not exist in the actual memory representation of the names used in the system call. In the system call there is just an N-byte buffer. If you have {0, 'h', 'e', 'l', 'l', 'o', 0, 0, 0, 0, ...} the convention is to print that as @hello. If you have {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ...} we currently print that as "@". The bug report is that we should instead print it as "". That's fine to fix, but the point is that there's only one all-zero name. Either all zeros == "@" or all zeros == "". The other is then an invalid name. Today all zeros is "@" and "" is invalid/unspecifiable. The issue is about reversing these.

[rsc, 2014-04-17 02:49:29]

R=close I think it's too late for 1.3. Feel free to hg mail after 1.3 for 1.4. The more I look at this the less sure I am that we need to do anything. It seems like it might be a feature that "" prints as "@". If we do want to change it, this seems like a simpler CL: g% hg diff diff -r 3f2fed7c0a19 src/pkg/net/unix_test.go --- a/src/pkg/net/unix_test.go	Wed Apr 16 22:43:44 2014 -0400 +++ b/src/pkg/net/unix_test.go	Wed Apr 16 22:49:16 2014 -0400 @@ -256,9 +256,6 @@	t.Fatalf("UnixConn.Write failed: %v", err)	} -	if runtime.GOOS == "linux" && laddr == "" { -	laddr = "@" // autobind feature -	}	var connAddrs = [3]struct{ got, want Addr }{	{ln.Addr(), ta},	{c.LocalAddr(), &UnixAddr{Name: laddr, Net: "unix"}}, @@ -308,9 +305,6 @@	}	}() -	if runtime.GOOS == "linux" && laddr == "" { -	laddr = "@" // autobind feature -	}	var connAddrs = [4]struct{ got, want Addr }{	{c1.LocalAddr(), ta},	{c1.RemoteAddr(), nil}, diff -r 3f2fed7c0a19 src/pkg/syscall/syscall_linux.go --- a/src/pkg/syscall/syscall_linux.go	Wed Apr 16 22:43:44 2014 -0400 +++ b/src/pkg/syscall/syscall_linux.go	Wed Apr 16 22:49:16 2014 -0400 @@ -352,7 +352,7 @@	case AF_UNIX:	pp := (*RawSockaddrUnix)(unsafe.Pointer(rsa))	sa := new(SockaddrUnix) -	if pp.Path[0] == 0 { +	if pp.Path[0] == 0 && pp.Path[1] != 0 {	// "Abstract" Unix domain socket.	// Rewrite leading NUL as @ for textual display.	// (This is the standard convention.) g%