[kudu-CR] KUDU-2351 Add IP/port for Recv() failure

["Todd Lipcon (Code Review)" <ge...@cloudera.org>]

Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9818 )

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


Patch Set 9:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc
File src/kudu/util/net/socket-test.cc:

http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@1
PS9, Line 1: 
nit: blank line at top of file


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@45
PS9, Line 45:   public:
style: indentation seems off in this file. we only indent private/public by 1 space, and then the stuff underneath them an additional 1 space, so that the total indentation is 2 within the class


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@46
PS9, Line 46:     virtual void SetUp() override {
            :       KuduTest::SetUp();
            :     }
is this necessary?


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@56
PS9, Line 56: }
again strange indent


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@63
PS9, Line 63: std::promise
hrm, we don't really use std::promise and std::future elsewhere in the codebase. Can you switch this over to Promise from util/promise.h for now? Maybe we should switch at some point but for now let's prefer consistency


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@73
PS9, Line 73: GetSocketAddress
should CHECK_OK this


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@76
PS9, Line 76:  Sockaddr new_addr;
            :         Socket sock;
these aren't needed in this scope - they can be inside 'if (accept)'


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@80
PS9, Line 80:           sock.Close();
also CHECK_OK


http://gerrit.cloudera.org:8080/#/c/9818/9/src/kudu/util/net/socket-test.cc@93
PS9, Line 93: SetRecvTimeout
here too



-- 
To view, visit http://gerrit.cloudera.org:8080/9818
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I22436b13bb351b132e1c0b7159294dd0c980c2b3
Gerrit-Change-Number: 9818
Gerrit-PatchSet: 9
Gerrit-Owner: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 04 Apr 2018 00:40:21 +0000
Gerrit-HasComments: Yes