net: Return error from CloseRead and CloseWrite.

net: Return error from CloseRead and CloseWrite.

[albert.strasheim, 2011-09-30 08:47:33]

Hello all

Here is the basic change to make CloseRead and CloseWrite return an error. I
only did fd.go for now, I'll do fd_windows.go too if we can address a few
questions I have:

1. Where is CloseReadWrite? If a user needs to Shutdown sockets, he might need
any one of the three possibilities exposed via the API.

2. Is Close really the best name? For me, having Close in the name means it does
something with the file descriptor, whereas Shutdown specifically does not close
the file descriptor.

Looking forward to your comments.

Regards

Albert

[iant2, 2011-09-30 13:15:43]

fullung@gmail.com writes:

> 1. Where is CloseReadWrite? If a user needs to Shutdown sockets, he
> might need any one of the three possibilities exposed via the API.

How does CloseReadWrite differ meaningfully from Close?


> -func (fd *netFD) CloseRead() os.Error {
> +func (fd *netFD) shutdown(how int) os.Error {
>  	if fd == nil || fd.sysfile == nil {
>  		return os.EINVAL
>  	}
> -	syscall.Shutdown(fd.sysfd, syscall.SHUT_RD)
> +	errno := syscall.Shutdown(fd.sysfd, how)
> +	var oserr os.Error
> +	if errno != 0 {
> +		oserr = os.Errno(errno)
> +	}
> +	if oserr != nil {
> +		return &OpError{"shutdown", fd.net, fd.laddr, oserr}
> +	}

How about
	if errno := syscall.Shutdown(fd.sysfd, how); errno != 0 {
		return &OpError("shutdown", fd.net, fd.laddr, os.Errno(errno))
	}

Ian

[albert.strasheim, 2011-09-30 13:44:13]

Hello

On Fri, Sep 30, 2011 at 3:15 PM, Ian Lance Taylor <iant@google.com> wrote:
> fullung@gmail.com writes:
>
>> 1. Where is CloseReadWrite? If a user needs to Shutdown sockets, he
>> might need any one of the three possibilities exposed via the API.
> How does CloseReadWrite differ meaningfully from Close?

You make a valid point. I'm just keen to avoid any confusion between
Shutdown and Close. Shutdown is a socket-level thing. Close operates
on file descriptors.

As soon as you start passing file descriptors between processes, it
becomes very important not to confuse the two.

>> -func (fd *netFD) CloseRead() os.Error {
>> +func (fd *netFD) shutdown(how int) os.Error {
>>       if fd == nil || fd.sysfile == nil {
>>               return os.EINVAL
>>       }
>> -     syscall.Shutdown(fd.sysfd, syscall.SHUT_RD)
>> +     errno := syscall.Shutdown(fd.sysfd, how)
>> +     var oserr os.Error
>> +     if errno != 0 {
>> +             oserr = os.Errno(errno)
>> +     }
>> +     if oserr != nil {
>> +             return &OpError{"shutdown", fd.net, fd.laddr, oserr}
>> +     }
>
> How about
>        if errno := syscall.Shutdown(fd.sysfd, how); errno != 0 {
>                return &OpError("shutdown", fd.net, fd.laddr, os.Errno(errno))
>        }

Will do.

Regards

Albert

[bradfitz, 2011-09-30 15:54:07]

On Fri, Sep 30, 2011 at 1:47 AM, <fullung@gmail.com> wrote:

> Reviewers: bradfitz, rsc,
>
> Message:
> Hello all
>
> Here is the basic change to make CloseRead and CloseWrite return an
> error. I only did fd.go for now, I'll do fd_windows.go too if we can
> address a few questions I have:
>
> 1. Where is CloseReadWrite? If a user needs to Shutdown sockets, he
> might need any one of the three possibilities exposed via the API.
>
> 2. Is Close really the best name? For me, having Close in the name means
> it does something with the file descriptor, whereas Shutdown
> specifically does not close the file descriptor.
>

I picked CloseRead and CloseWrite because they were shorter than
ShutdownRead and ShutdownWrite.  If I added Shutdown(flags int), then I'd
need to define flags somewhere.  *shrug*

I don't care too much if people want to rename them Shutdown{Read,Write} but
I'd rather not have the flags version.

It sounds like you want shutdown more than I did and know more so I'll defer
to you.  I just wanted it for completeness and for this test:

http://code.google.com/p/go/source/detail?r=b1e0821538e1e29ee0ff431531e6516ba...




>
> Looking forward to your comments.
>
> Regards
>
> Albert
>
> Description:
> net: Return error from CloseRead and CloseWrite.
>
> Please review this at
http://codereview.appspot.com/**5167043/<http://codereview.appspot.com/5167043/>
>
> Affected files:
>  M src/pkg/net/fd.go
>
>
> Index: src/pkg/net/fd.go
> ==============================**==============================**=======
> --- a/src/pkg/net/fd.go
> +++ b/src/pkg/net/fd.go
> @@ -358,20 +358,27 @@
>        return nil
>  }
>
> -func (fd *netFD) CloseRead() os.Error {
> +func (fd *netFD) shutdown(how int) os.Error {
>        if fd == nil || fd.sysfile == nil {
>                return os.EINVAL
>        }
> -       syscall.Shutdown(fd.sysfd, syscall.SHUT_RD)
> +       errno := syscall.Shutdown(fd.sysfd, how)
> +       var oserr os.Error
> +       if errno != 0 {
> +               oserr = os.Errno(errno)
> +       }
> +       if oserr != nil {
> +               return &OpError{"shutdown", fd.net, fd.laddr, oserr}
> +       }
>        return nil
>  }
>
> +func (fd *netFD) CloseRead() os.Error {
> +       return fd.shutdown(syscall.SHUT_RD)
> +}
> +
>  func (fd *netFD) CloseWrite() os.Error {
> -       if fd == nil || fd.sysfile == nil {
> -               return os.EINVAL
> -       }
> -       syscall.Shutdown(fd.sysfd, syscall.SHUT_WR)
> -       return nil
> +       return fd.shutdown(syscall.SHUT_WR)
>  }
>
>  func (fd *netFD) Read(p []byte) (n int, err os.Error) {
>
>
>

[rsc, 2011-10-12 17:44:52]

LGTM (without CloseReadWrite)

[rsc, 2011-10-12 17:45:27]

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

net: Return error from CloseRead and CloseWrite.

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

Committer: Russ Cox <rsc@golang.org>