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%