|
|
Created:
12 years, 2 months ago by mikio Modified:
12 years, 2 months ago Reviewers:
CC:
golang-dev, bradfitz Visibility:
Public. |
Descriptionnet: fix linux build
Patch Set 1 #Patch Set 2 : diff -r b60bc9a6b439 https://go.googlecode.com/hg/ #Patch Set 3 : diff -r b60bc9a6b439 https://go.googlecode.com/hg/ #Patch Set 4 : diff -r 25879029ed5b https://go.googlecode.com/hg/ #Patch Set 5 : diff -r 25879029ed5b https://go.googlecode.com/hg/ #
Total comments: 1
Patch Set 6 : diff -r cc1fb54d8263 https://go.googlecode.com/hg/ #MessagesTotal messages: 14
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
Is that a fix or just silencing the test? On Thu, Jan 19, 2012 at 3:45 PM, <mikioh.mikioh@gmail.com> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), > > I'd like you to review this change to > https://go.googlecode.com/hg/ > > > Description: > net: fix linux build > > Please review this at http://codereview.appspot.com/**5558056/<http://codereview.appspot.com/5558056/> > > Affected files: > M src/pkg/net/unicast_test.go > > > Index: src/pkg/net/unicast_test.go > ==============================**==============================**======= > --- a/src/pkg/net/unicast_test.go > +++ b/src/pkg/net/unicast_test.go > @@ -35,6 +35,9 @@ > } > var fd *netFD > if !tt.packet { > + if runtime.GOOS == "linux" { > + tt.laddr = "" > + } > if tt.laddr == "previous" { > tt.laddr = prevladdr > } > > >
Sign in to reply to this message.
If it's a fix, it needs a comment inside that block, but pretty sad that a caller of the net package (in this, a test) needs to care about runtime.GOOS. On Thu, Jan 19, 2012 at 3:47 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > Is that a fix or just silencing the test? > > > > On Thu, Jan 19, 2012 at 3:45 PM, <mikioh.mikioh@gmail.com> wrote: > >> Reviewers: golang-dev_googlegroups.com, >> >> Message: >> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), >> >> I'd like you to review this change to >> https://go.googlecode.com/hg/ >> >> >> Description: >> net: fix linux build >> >> Please review this at http://codereview.appspot.com/**5558056/<http://codereview.appspot.com/5558056/> >> >> Affected files: >> M src/pkg/net/unicast_test.go >> >> >> Index: src/pkg/net/unicast_test.go >> ==============================**==============================**======= >> --- a/src/pkg/net/unicast_test.go >> +++ b/src/pkg/net/unicast_test.go >> @@ -35,6 +35,9 @@ >> } >> var fd *netFD >> if !tt.packet { >> + if runtime.GOOS == "linux" { >> + tt.laddr = "" >> + } >> if tt.laddr == "previous" { >> tt.laddr = prevladdr >> } >> >> >> >
Sign in to reply to this message.
It's just a pray for silence. We need to have a look at the kernel code a bit more, how they reuse twsk (how can I imagine twsk stands for tcp-timewait-socket) stuff. On Fri, Jan 20, 2012 at 8:48 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > If it's a fix, it needs a comment inside that block, but pretty sad that a > caller of the net package (in this, a test) needs to care about > runtime.GOOS. Yup, same as diat_test.go.
Sign in to reply to this message.
On Fri, Jan 20, 2012 at 10:22 AM, Mikio Hara <mikioh.mikioh@gmail.com> wrote: > We need to have a look at the kernel code a bit more, how they reuse > twsk (how can I imagine twsk stands for tcp-timewait-socket) stuff. Nope, I misunderstood, it's a listening socket. The only thing we need to do is... aha! drop defer. // I'm not sure, not saying the behavior of defer is different // btw linux and others. But I suspect... Anyway, it's fixed, PTAL.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
Glad you found it! http://codereview.appspot.com/5558056/diff/7001/src/pkg/net/unicast_test.go File src/pkg/net/unicast_test.go (right): http://codereview.appspot.com/5558056/diff/7001/src/pkg/net/unicast_test.go#n... src/pkg/net/unicast_test.go:17: l Listener why do you need this in the test case? in the test function itself, can't you just do: var closer io.Closer closer = .. closer = ... closer.Close()
Sign in to reply to this message.
On Fri, Jan 20, 2012 at 11:31 AM, <bradfitz@golang.org> wrote: > Glad you found it! Thank you. > http://codereview.appspot.com/5558056/diff/7001/src/pkg/net/unicast_test.go#n... > src/pkg/net/unicast_test.go:17: l Listener > why do you need this in the test case? Ah, I can't recall io.Closer interface, thx.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
LGTM Thanks! It's back to passing here. On Thu, Jan 19, 2012 at 6:47 PM, <mikioh.mikioh@gmail.com> wrote: > Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: > golang-dev@googlegroups.com), > > Please take another look. > > > http://codereview.appspot.com/**5558056/<http://codereview.appspot.com/5558056/> >
Sign in to reply to this message.
Incidentally, this is interesting data for a mystery bug of mine, http://code.google.com/p/go/issues/detail?id=2581 I guess that's just how these operating systems differ. On Thu, Jan 19, 2012 at 6:50 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > LGTM > > Thanks! It's back to passing here. > > > On Thu, Jan 19, 2012 at 6:47 PM, <mikioh.mikioh@gmail.com> wrote: > >> Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: >> golang-dev@googlegroups.com), >> >> Please take another look. >> >> >> http://codereview.appspot.com/**5558056/<http://codereview.appspot.com/5558056/> >> > >
Sign in to reply to this message.
*** Submitted as http://code.google.com/p/go/source/detail?r=566110b6be97 *** net: fix linux build R=golang-dev, bradfitz CC=golang-dev http://codereview.appspot.com/5558056 Committer: Brad Fitzpatrick <bradfitz@golang.org>
Sign in to reply to this message.
On Fri, Jan 20, 2012 at 11:52 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Incidentally, this is interesting data for a mystery bug of mine, > > http://code.google.com/p/go/issues/detail?id=2581 > > I guess that's just how these operating systems differ. Recently I realized that that's a part of Go's fun even if I have to write a patch on a crush commuter train.
Sign in to reply to this message.
On Thu, Jan 19, 2012 at 7:22 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote: > On Fri, Jan 20, 2012 at 11:52 AM, Brad Fitzpatrick <bradfitz@golang.org> > wrote: > > > Incidentally, this is interesting data for a mystery bug of mine, > > > > http://code.google.com/p/go/issues/detail?id=2581 > > > > I guess that's just how these operating systems differ. > > Recently I realized that that's a part of Go's fun even if > I have to write a patch on a crush commuter train. > Glad you got the files uploaded before you hit the tunnel and lost connectivity, all Mission Impossible-style. :-)
Sign in to reply to this message.
|