Dan Hecht has posted comments on this change. Change subject: IMPALA-5221: Fix TSaslTransport negotiation order ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/7116/1/be/src/transport/TSasl.h File be/src/transport/TSasl.h: PS1, Line 69: virtual why is this one virtual? and how about disposeSaslContext()? Line 71: conn = NULL; doesn't this need to be wrapped in "if (conn != nullptr)"? i.e. can't we get here with conn already nullptr? Line 131: /* List of registered callbacks */ who owns it? and any way to be more descriptive than "registered callbacks"? Maybe at least "Callbacks registered with the Sasl connection"? -- To view, visit http://gerrit.cloudera.org:8080/7116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d509e14dfe46fa28d0626cf84daf6de82955d80 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
