code review 6445070: net: enable File method for sockets on windows

net: enable File method for sockets on windows

This CL implements dup method on netFD and makes it possible
to use File method for ProtocolConn on Windows.

Fixes issue 3827.

[brainman, 2012-08-12 08:33:24]

Unfortunately, the tests fail:

...
=== RUN TestFileListener
--- FAIL: TestFileListener (0.02 seconds)
file_test.go:46: 	FileListener failed: FileListener: not supported by windows
=== RUN TestFilePacketConn
--- FAIL: TestFilePacketConn (0.00 seconds)
file_test.go:115: 	FilePacketConn failed: FilePacketConn: not supported by
windows
...

Alex

[mikio, 2012-08-14 11:01:45]

On 2012/08/12 08:33:24, brainman wrote:

> Unfortunately, the tests fail:

Ouch, thanks for your confirmation.
I'll add an another test and send CL to the review.

[mikio, 2012-08-14 11:37:24]

Hello alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[brainman, 2012-08-15 06:46:22]

TestFileMethod pass on windows. But I do not see it doing anything useful there.
What is the purpose of adding this functionality? How can someone use it for
anything?

Alex

[mikio, 2012-08-15 11:11:45]

On Wed, Aug 15, 2012 at 3:46 PM,  <alex.brainman@gmail.com> wrote:

> TestFileMethod pass on windows. But I do not see it doing anything
> useful there. What is the purpose of adding this functionality? How can
> someone use it for anything?

It allows opeating IP-level socket options from outside.

func main() {
    c, err := Dial()
    f, err = c.File()
    syscall.SetsockoptInt(int(f.Fd()), syscall.IPPROTO_IP,
syscall.IP_RECVDSTADDR, 1)
}

[minux1, 2012-08-15 16:08:18]

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

http://codereview.appspot.com/6445070/diff/7001/src/pkg/net/fd_windows.go#new...
src/pkg/net/fd_windows.go:574: // Unimplemented functions.
dup() is no longer unimplemented function, please move this comment down

[mikio, 2012-08-15 17:11:02]

On 2012/08/15 16:08:18, minux wrote:

> src/pkg/net/fd_windows.go:574: // Unimplemented functions.
> dup() is no longer unimplemented function, please move this comment down

pls have a look at #2 but thx.

[minux1, 2012-08-15 17:28:16]

On Thu, Aug 16, 2012 at 1:11 AM, <mikioh.mikioh@gmail.com> wrote:

> On 2012/08/15 16:08:18, minux wrote:
>
>> src/pkg/net/fd_windows.go:574: // Unimplemented functions.
>> dup() is no longer unimplemented function, please move this comment
>>
> down
>
> pls have a look at #2 but thx.
>
oops, sorry, my comment arrived late.

[brainman, 2012-08-15 23:53:18]

On 2012/08/15 11:11:45, mikio wrote:
> ...
> 
> It allows opeating IP-level socket options from outside.
> 
> func main() {
>     c, err := Dial()
>     f, err = c.File()
>     syscall.SetsockoptInt(int(f.Fd()), syscall.IPPROTO_IP,
> syscall.IP_RECVDSTADDR, 1)
> }

Does it? Lets see some tests to demonstrate that. Otherwise, it just a wishful
thinking on our part. How do we know that duplicate of our original handle will
provide all these things that we expect it to provide?

And secondly, if all we are trying to do here is get access to socket handle,
then perhaps it is a roundabout way of getting to it. Why this

c, err := Dial(...)
if err != nil {
	return err
}
defer c.Close()
f, err := c.File()
if err != nil {
	return err
}
defer f.Close()
println(f.Fd())

is better, then this

c, err := Dial(...)
if err != nil {
	return err
}
defer c.Close()
println(c.Handle())

?

Alex

[mikio, 2012-08-16 10:16:37]

Hello alex.brainman@gmail.com, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[mikio, 2012-08-16 10:40:15]

PTAL

On 2012/08/15 23:53:18, brainman wrote:

> Does it? Lets see some tests to demonstrate that.

Sure, done.

> And secondly, if all we are trying to do here is get access to socket handle,
> then perhaps it is a roundabout way of getting to it.

Does the same signature on various ProtocolConn across over 
platforms make sense? Also I'd like to use existing abstraction
layers we already have like below.

net.Conn over os.File over uintptr over int/uint32 for posix or syscall.Handle
for windows.

[brainman, 2012-08-17 06:38:35]

On 2012/08/16 10:40:15, mikio wrote:
> PTAL
> 

test does not compile now:

# GOOS=windows go test -c
# net
./file_test.go:220: cannot use int(lnf.Fd()) (type int) as type syscall.Handle
in function argument
./file_test.go:233: cannot use int(cf.Fd()) (type int) as type syscall.Handle in
function argument

> 
> > Does it? Lets see some tests to demonstrate that.
> 
> Sure, done.
> 

I do not think so. All your new test does is call syscall and check for result.
If you test changes some socket option, I want the test to demonstrate the
effect of that change, in one way or the other.

> > And secondly, if all we are trying to do here is get access to socket
handle,
> > then perhaps it is a roundabout way of getting to it.
> 
> Does the same signature on various ProtocolConn across over 
> platforms make sense? ...

Sure, but having (...).Handle() function to return socket handle does the job
and simple at the same time.

> ... Also I'd like to use existing abstraction
> layers we already have like below.
> 
> net.Conn over os.File over uintptr over int/uint32 for posix or syscall.Handle
> for windows.

I do not see the benefit. I am also not convinced yet that exposing "duplicate"
handle will provide required functionality.

Alex

[mikio, 2012-08-21 12:17:21]

On 2012/08/17 06:38:35, brainman wrote:

> test does not compile now:

oops, sorry.

> I do not think so. All your new test does is call syscall and check for
result.
> If you test changes some socket option, I want the test to demonstrate the
> effect of that change, in one way or the other.

hmm, let me sleep on it.

> I do not see the benefit. I am also not convinced yet that exposing
"duplicate"
> handle will provide required functionality.

[mikio, 2012-09-04 03:39:12]

I give up, will abandon.

also rsc suggests to using reflect to dig up underlying
unix file descriptor or windows handle, I'll go this way.

[mikio, 2012-09-04 03:40:29]

*** Abandoned ***

[anacrolix, 2012-09-06 14:03:14]

On 2012/09/04 03:40:29, mikio wrote:
> *** Abandoned ***

@mikio: What was the reason for giving up? Process too difficult or did your
proposed solution not work?

[mikio, 2012-09-06 14:13:23]

On Thu, Sep 6, 2012 at 11:03 PM,  <anacrolix@gmail.com> wrote:

> @mikio: What was the reason for giving up? Process too difficult or did
> your proposed solution not work?

The latter. I need the underlying socket descriptor for socket options.
In the review process Russ pointed out:

<http://codereview.appspot.com/6482044/>
Please don't use .File() to get a file descriptor. That will disable
non-blocking I/O on the connection. The fd is intentionally hidden, precisely
to
keep people from touching it behind package net's back. To get around that I
would suggest using reflect to dig into the structs and extract the fd integer.