FAQ
Reviewers: golang-dev1,

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

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


Description:
net: add missing DialTCP argument test

Please review this at https://codereview.appspot.com/8859050/

Affected files:
    M src/pkg/net/protoconn_test.go


Index: src/pkg/net/protoconn_test.go
===================================================================
--- a/src/pkg/net/protoconn_test.go
+++ b/src/pkg/net/protoconn_test.go
@@ -97,7 +97,7 @@
    if err != nil {
     t.Fatalf("ResolveTCPAddr failed: %v", err)
    }
- c, err := DialTCP("tcp4", nil, ra)
+ c, err := DialTCP("tcp4", la, ra)
    if err != nil {
     t.Fatalf("DialTCP failed: %v", err)
    }


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Dave at Apr 27, 2013 at 3:48 pm

    On 2013/04/27 15:37:38, mikio wrote:
    Hello mailto:golang-dev@googlegroups.com (cc:
    mailto:golang-dev@googlegroups.com),
    I'd like you to review this change to
    https://code.google.com/p/go
    Hi Mikio,

    What is this testing ? From my reading it dials ra using the source
    address of la, but la is already bound by a ListenTCP above. Not that I
    have ever tried, but does this work ?

    Dave

    https://codereview.appspot.com/8859050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Mikio Hara at Apr 27, 2013 at 4:00 pm

    On Sun, Apr 28, 2013 at 12:48 AM, wrote:

    What is this testing ? From my reading it dials ra using the source
    address of la, but la is already bound by a ListenTCP above.
    ResolveTCPAddr just returns a TCPAddr.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dave Cheney at Apr 27, 2013 at 4:06 pm
    Ok. I think I understand now. Would you please update the description
    with something along these lines (assuming it is correct)

    "Ensure TestTCPConnSpecificMethods chooses the loopback interface as
    both the source and the target."
    On Sun, Apr 28, 2013 at 2:00 AM, Mikio Hara wrote:
    On Sun, Apr 28, 2013 at 12:48 AM, wrote:

    What is this testing ? From my reading it dials ra using the source
    address of la, but la is already bound by a ListenTCP above.
    ResolveTCPAddr just returns a TCPAddr.
    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Mikio Hara at Apr 27, 2013 at 4:16 pm

    On Sun, Apr 28, 2013 at 1:06 AM, Dave Cheney wrote:

    Ok. I think I understand now. Would you please update the description
    with something along these lines (assuming it is correct)

    "Ensure TestTCPConnSpecificMethods chooses the loopback interface as
    both the source and the target."
    Thanks but no thanks.

    It just tests DialTCP with both destination and local addresses,
    nothing more or less. Also we cannot ensure that the test will use
    loopback interface because it depends on the platform implementation.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Alex Brainman at Apr 27, 2013 at 11:40 pm
    I am not an expert in TCP, but I think this is wrong. la represents an
    IP/port pair that our listener is listening on. Can it be used as local
    IP/port pair for a new TCP connection?

    I also suspect that this will break on windows, similar to what is
    happening in issue 5355. So this could be a test for issue 5355. But you
    should both scenarios: la is nil and la is not nil.

    Alex

    https://codereview.appspot.com/8859050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Mikio Hara at May 10, 2013 at 2:51 pm
    Thanks for fixing issues related to Dial stuff.
    Will abandon this CL and revisit later for refactoring testcases.
    On Sun, Apr 28, 2013 at 8:40 AM, wrote:

    I am not an expert in TCP, but I think this is wrong. la represents an
    IP/port pair that our listener is listening on. Can it be used as local
    IP/port pair for a new TCP connection?
    That variable represents "127.0.0.1:0", a pair of specific IP address and
    wildcard port. Both Listen and Dial functions never tweak their parameters.
    (Also POSIX and Windows socket API requires syscall.Sockaddrs instead
    of net.ProtocolAddrs.)
    I also suspect that this will break on windows, similar to what is
    happening in issue 5355. So this could be a test for issue 5355. But you
    should both scenarios: la is nil and la is not nil.
    Yup.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Alex Brainman at Apr 27, 2013 at 11:42 pm
    But you TEST should both
    scenarios: la is nil and la is not nil. (phone fingers).

    https://codereview.appspot.com/8859050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Mikioh Mikioh at May 10, 2013 at 2:52 pm
    *** Abandoned ***

    https://codereview.appspot.com/8859050/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedApr 27, '13 at 3:37p
activeMay 10, '13 at 2:52p
posts9
users3
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase