Todd Lipcon has posted comments on this change. Change subject: KuduRPC integration with OpenSSL ......................................................................
Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4789/2/src/kudu/util/net/ssl_factory.cc File src/kudu/util/net/ssl_factory.cc: PS2, Line 29: Mutex* > Consider using std::mutex instead. Also, what is the reason of allocating I disagree with the std::mutex thing - we don't typically use those, and instead use our own. But agreed that pointers are strange... I guess this is because these are purposefully leaked mutexes and we don't want non-POD values in static storage. It would be worth a comment to that effect. PS2, Line 42: return pthread_self(); > This is not portable: pthread_t can be any type (like a structure). Instea I think we have some wrapper in Thread which is a portable way of getting a unique tid. I don't think the hasher approach guarantees any uniqueness. -- To view, visit http://gerrit.cloudera.org:8080/4789 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I27167faa4e6a78e59b46093055b16682c93af0ea Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
