----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71660/#review218573 -----------------------------------------------------------
3rdparty/libprocess/src/CMakeLists.txt Lines 101-103 (patched) <https://reviews.apache.org/r/71660/#comment306309> Need to update autotools build also? Or is this Windows-only for now? 3rdparty/libprocess/src/ssl/socket_wrapper.hpp Lines 24-28 (patched) <https://reviews.apache.org/r/71660/#comment306310> What do you think about making `SSLSocketWrapper` a true wrapper of the underlying socket? In other words, let's make the wrapper a subclass of `SocketImpl` and have it hold a `SocketImpl` member which is the underlying socket that it's using for communication. We could add a `create()` method and a constructor like this: ``` Try<std::shared_ptr<SocketImpl>> SSLSocketWrapper::create(const SocketImpl& socketImpl_) { openssl::initialize(); if (!openssl::flags().enabled) { return Error("SSL is disabled"); } return std::make_shared<SSLSocketWrapper>(socketImpl_); } SSLSocketWrapper::SSLSocketWrapper(const SocketImpl& socketImpl_) : socketImpl(socketImpl_), ssl(nullptr) {} ``` then we could call methods like `connect()` and `accept()` directly on the held socket within the wrapper code. It's a small structural change, but I think it would help to communicate to readers how the underlying socket and the SSL wrapper are related. Further, it makes it more obvious how we could evolve the wrapper class into some kind of `Server` and `Client` objects which are initialized with a socket. WDYT? - Greg Mann On Oct. 24, 2019, 1:41 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71660/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2019, 1:41 a.m.) > > > Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till > Toenshoff. > > > Bugs: MESOS-10009 > https://issues.apache.org/jira/browse/MESOS-10009 > > > Repository: mesos > > > Description > ------- > > This creates some new files implementing the SocketImpl class, > in preparation for implementing generic SSL sockets. > > The new class is used whenever SSL is enabled, but Libevent is not. > > > Diffs > ----- > > 3rdparty/libprocess/src/CMakeLists.txt > 40c8ef935104bab4ea2f8b2b5919450b55165f60 > 3rdparty/libprocess/src/openssl.cpp > bd05866950e1043d9585a7c5fdc7b2147a233fd3 > 3rdparty/libprocess/src/socket.cpp 606a1c46e50936251c29af4b996007c480b2a135 > 3rdparty/libprocess/src/ssl/socket_wrapper.hpp PRE-CREATION > 3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/71660/diff/2/ > > > Testing > ------- > > cmake .. -ENABLE_SSL=1 # No Libevent here! > > > Thanks, > > Joseph Wu > >
