code review 95320043: net: fix documentation for SetLinger

net: fix documentation for SetLinger

Fixes issue 7974.

[mikio, 2014-05-12 17:18:06]

Hello golang-codereviews@googlegroups.com (cc:
golang-codereviews@googlegroups.com),

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

[iant, 2014-05-12 17:59:43]

https://codereview.appspot.com/95320043/diff/40001/src/pkg/net/tcpsock_posix.go
File src/pkg/net/tcpsock_posix.go (right):

https://codereview.appspot.com/95320043/diff/40001/src/pkg/net/tcpsock_posix....
src/pkg/net/tcpsock_posix.go:102: // If sec > 0, Close returns immediately and
the operating system
I think that both the original comment and your proposed revision are incorrect.
 I think that SO_LINGER with a non-zero value has no effect on a nonblocking
socket.  The close simply returns immediately, and the linger time has no
meaningful effect.

[mikio, 2014-05-13 03:34:42]

On Tue, May 13, 2014 at 2:59 AM,  <iant@golang.org> wrote:

> I think that both the original comment and your proposed revision are
> incorrect.  I think that SO_LINGER with a non-zero value has no effect
> on a nonblocking socket.  The close simply returns immediately, and the
> linger time has no meaningful effect.

haven't looked linux or others yet but looks so. thanks for the
notice. hm, what about this?

// If sec > 0, Close returns immediately and the operating system
// finishes sending the data in background, until sec or protocol timer
// elapses.

[iant, 2014-05-13 05:07:47]

On Mon, May 12, 2014 at 8:34 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> On Tue, May 13, 2014 at 2:59 AM,  <iant@golang.org> wrote:
>
>> I think that both the original comment and your proposed revision are
>> incorrect.  I think that SO_LINGER with a non-zero value has no effect
>> on a nonblocking socket.  The close simply returns immediately, and the
>> linger time has no meaningful effect.
>
> haven't looked linux or others yet but looks so. thanks for the
> notice. hm, what about this?
>
> // If sec > 0, Close returns immediately and the operating system
> // finishes sending the data in background, until sec or protocol timer
> // elapses.

I don't really see how there is anything useful to say if sec > 0.  As
far as I can tell the value is simply ignored.  There is effectively
no difference between sec < 0 and sec > 0.  At least, I don't see how
to detect a difference.

Ian

[mikio, 2014-05-13 05:23:30]

On Tue, May 13, 2014 at 2:07 PM, Ian Lance Taylor <iant@golang.org> wrote:

> I don't really see how there is anything useful to say if sec > 0.  As
> far as I can tell the value is simply ignored.  There is effectively
> no difference between sec < 0 and sec > 0.  At least, I don't see how
> to detect a difference.

agree, but we can't change setlinger api, follow kernel/protocol stack
changes in the future. are you suggesting we should say more
explicitly like "for now almost all the protocol stack implementations
ignore linger time value"?

[iant, 2014-05-13 05:55:37]

On Mon, May 12, 2014 at 10:23 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> On Tue, May 13, 2014 at 2:07 PM, Ian Lance Taylor <iant@golang.org> wrote:
>
>> I don't really see how there is anything useful to say if sec > 0.  As
>> far as I can tell the value is simply ignored.  There is effectively
>> no difference between sec < 0 and sec > 0.  At least, I don't see how
>> to detect a difference.
>
> agree, but we can't change setlinger api, follow kernel/protocol stack
> changes in the future. are you suggesting we should say more
> explicitly like "for now almost all the protocol stack implementations
> ignore linger time value"?

I guess so.  How about this?

// SetLinger sets the behavior of Close on a connection which still
// has data waiting to be sent or to be acknowledged.
//
// If sec < 0 (the default), the operating system finishes sending the
// data in the background.
//
// If sec == 0, the operating system discards any unsent or
// unacknowledged data.
//
// If sec > 0, the data is sent in the background as with sec < 0.  On
// some operating systems after sec seconds have elapsed any remaining
// unsent data may be discarded.

[mikio, 2014-05-13 06:17:09]

much better, thanks.


On Tue, May 13, 2014 at 2:55 PM, Ian Lance Taylor <iant@golang.org> wrote:
> On Mon, May 12, 2014 at 10:23 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
>> On Tue, May 13, 2014 at 2:07 PM, Ian Lance Taylor <iant@golang.org> wrote:
>>
>>> I don't really see how there is anything useful to say if sec > 0.  As
>>> far as I can tell the value is simply ignored.  There is effectively
>>> no difference between sec < 0 and sec > 0.  At least, I don't see how
>>> to detect a difference.
>>
>> agree, but we can't change setlinger api, follow kernel/protocol stack
>> changes in the future. are you suggesting we should say more
>> explicitly like "for now almost all the protocol stack implementations
>> ignore linger time value"?
>
> I guess so.  How about this?
>
> // SetLinger sets the behavior of Close on a connection which still
> // has data waiting to be sent or to be acknowledged.
> //
> // If sec < 0 (the default), the operating system finishes sending the
> // data in the background.
> //
> // If sec == 0, the operating system discards any unsent or
> // unacknowledged data.
> //
> // If sec > 0, the data is sent in the background as with sec < 0.  On
> // some operating systems after sec seconds have elapsed any remaining
> // unsent data may be discarded.

[mikio, 2014-05-13 06:19:11]

Hello golang-codereviews@googlegroups.com, iant@golang.org (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[iant, 2014-05-13 17:02:34]

LGTM

[mikio, 2014-05-14 03:12:06]

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

net: fix documentation for SetLinger

Fixes issue 7974.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/95320043

[gobot, 2014-05-14 03:39:25]

This CL appears to have broken the plan9-386-cnielsen builder.
See http://build.golang.org/log/283b996102b833dd81c58301d78aceaa4fe9838b

[mikio, 2014-05-14 04:12:47]

looks lile i parked a wrong spot. in this case skip=4 is not good for plan9.

On Wednesday, May 14, 2014 12:39:25 PM UTC+9, go...@golang.org wrote:
>
> This CL appears to have broken the plan9-386-cnielsen builder. 
> See http://build.golang.org/log/283b996102b833dd81c58301d78aceaa4fe9838b 
>
> https://codereview.appspot.com/95320043/ 
>