Alexey Serbin has posted comments on this change. Change subject: Workaround test failures running with MIT krb5 1.10 ......................................................................
Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/init.cc File src/kudu/security/init.cc: PS5, Line 88: krb5_creds creds > ah, good catch, I think you're right. Wonder why LSAN didn't catch this Interesting, indeed. May be, this is the case when the creds do not contain much at all? PS5, Line 112: "KRB5CCNAME" Does it make sense to introduce constants for those Kerberos-related env variables? It seems we have those string literals in multiple places now. At least, it could help to avoid mistakes related to typos/etc. http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/test/krb5_realm_override.cc File src/kudu/security/test/krb5_realm_override.cc: PS5, Line 78: int nit: they have krb5_error_code type for that, if you wish. PS5, Line 92: ret_realms nit: probably it's too paranoid, but does it make sense to check for non-null result of malloc(), and, may be, strdup() as well (since they are plain C functions and don't throw std::bad_alloc if no memory available)? -- To view, visit http://gerrit.cloudera.org:8080/4990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
