Todd Lipcon has posted comments on this change. Change subject: [security] groundwork for cert signing service ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5671/4/src/kudu/security/ca/cert_management.cc File src/kudu/security/ca/cert_management.cc: PS4, Line 632: : : per comment on the earlier revision, can you make this look less like a bug by doing something like: // X509v3 is, surprisingly, indicated by the value '2'. const int kX509V3 = 2; CERT_RET_NOT_OK(X509_set_version(ret, kX509V3)); http://gerrit.cloudera.org:8080/#/c/5671/1/src/kudu/security/crypto/cert_management.h File src/kudu/security/crypto/cert_management.h: PS1, Line 174: // The configuration and hostnames/ips are made separate parameters to : // work around limitations of old gcc 4.4.x compiler. Ideally, both hostnames : // and ips should have been fields of the Config structure. > I had some issues compiling the described alternative on ve0518.halxg.cloud hrm, you removed the comment, but now it doesn't really make sense why it's a separate parameter. Keep in mind that ve0518 is an el6 box, and so you need to be building with devtoolset. Were you trying to build this with te standard system compiler? Was the issue with the aggregate initialization you are doing elsewhere? iirc that isn't actually valid C++ syntax, so maybe we should just fix it there? Still seems weird that these are separate from the config. -- To view, visit http://gerrit.cloudera.org:8080/5671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic2ce55d38f4d06172fadaaa702f4550997d9bc8f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
