Todd Lipcon has posted comments on this change. ( )

Change subject: KUDU-2351 Add IP/port for Recv() failure

Patch Set 6:

File src/kudu/util/net/
PS6, Line 48:       address.ParseString("", 11010);
> address is used to check the error message, + I need to bind to something.
Can't you bind to port 0 so it picks an empty one, and then report back the 
bound address of the socket from the listener thread back to the main thread?
PS6, Line 53:         thread.join();
nit: too much indentation
PS6, Line 59: protected:
nit: indent by 1
PS6, Line 61:   Sockaddr address;
style: this member should end in _
PS6, Line 64: do_test
style: DoTest since this is a non-inline method
PS6, Line 66:    listener_.Init(0);
            :           listener_.BindAndListen(address, 0);
            :             Sockaddr new_addr;
            :             Socket sock;
nit: messy inentation. Please add CHECK_OK around all the call sites that 
return a Status in case they fail (also below in a few places)
PS6, Line 75:             
we use SleepFor from monotime.h instead
PS6, Line 82:       ASSERT_OK(client.Connect(address));
isn't this racey? how do we know that the other thread is listening yet? I 
think the other thread should set a Promise<Sockaddr> with its listening 
address after binding to port 0, and then you can wait on that promise here to 
find out where to connect
PS6, Line 91:       ASSERT_STR_CONTAINS(s.message().ToString(), message);
in order to deal with the ephemeral port you can use ASSERT_STR_MATCHES instead
PS6, Line 95: TEST_F(SocketTest, TestRecvReset) {
> I looked into it but I'm not sure if this is what I should use here as I ne
yea I think for this case it's easier to just do what he's done here

To view, visit
To unsubscribe, visit

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22436b13bb351b132e1c0b7159294dd0c980c2b3
Gerrit-Change-Number: 9818
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <>
Gerrit-Reviewer: Attila Bukor <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-Comment-Date: Tue, 27 Mar 2018 22:07:41 +0000
Gerrit-HasComments: Yes

Reply via email to