[jira] [Commented] (GEODE-8102) Link and load OpenSSL library directly

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GEODE-8102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17169210#comment-17169210 ] 

ASF GitHub Bot commented on GEODE-8102:
---------------------------------------

moleske commented on a change in pull request #630:
URL: https://github.com/apache/geode-native/pull/630#discussion_r463855720



##########
File path: cppcache/src/TcpConn.cpp
##########
@@ -126,142 +124,68 @@ void TcpConn::init() {
   connect();
 }
 
-TcpConn::TcpConn(const char *ipaddr, std::chrono::microseconds waitSeconds,
-                 int32_t maxBuffSizePool)
-    : m_io(nullptr),
-      m_addr(ipaddr),
-      m_waitMilliSeconds(waitSeconds),
-      m_maxBuffSizePool(maxBuffSizePool),
-      m_chunkSize(getDefaultChunkSize()) {}
-
-TcpConn::TcpConn(const char *hostname, int32_t port,
+TcpConn::TcpConn(const std::string& address,
                  std::chrono::microseconds waitSeconds, int32_t maxBuffSizePool)
-    : m_io(nullptr),
-      m_addr(port, hostname),
-      m_waitMilliSeconds(waitSeconds),
-      m_maxBuffSizePool(maxBuffSizePool),
-      m_chunkSize(getDefaultChunkSize()) {}
-
-void TcpConn::listen(const char *hostname, int32_t port,
-                     std::chrono::microseconds waitSeconds) {
-  ACE_INET_Addr addr(port, hostname);
-  listen(addr, waitSeconds);
-}
-
-void TcpConn::listen(const char *ipaddr,
-                     std::chrono::microseconds waitSeconds) {
-  ACE_INET_Addr addr(ipaddr);
-  listen(addr, waitSeconds);
-}
-
-void TcpConn::listen(ACE_INET_Addr addr,
-                     std::chrono::microseconds waitSeconds) {
-  using apache::geode::internal::chrono::duration::to_string;
-
-  ACE_SOCK_Acceptor listener(addr, 1);
-  int32_t retVal = 0;
-  if (waitSeconds > std::chrono::microseconds::zero()) {
-    ACE_Time_Value wtime(waitSeconds);
-    retVal = listener.accept(*m_io, nullptr, &wtime);
-  } else {
-    retVal = listener.accept(*m_io, nullptr);
-  }
-  if (retVal == -1) {
-    char msg[256];
-    int32_t lastError = ACE_OS::last_error();
-    if (lastError == ETIME || lastError == ETIMEDOUT) {
-      throw TimeoutException(
-          "TcpConn::listen Attempt to listen timed out after " +
-          to_string(waitSeconds) + ".");
-    }
-    std::snprintf(msg, 256, "TcpConn::listen failed with errno: %d: %s",
-                  lastError, ACE_OS::strerror(lastError));
-    throw GeodeIOException(msg);
-  }
-}
-
-void TcpConn::connect(const char *hostname, int32_t port,
-                      std::chrono::microseconds waitSeconds) {
-  ACE_INET_Addr addr(port, hostname);
-  m_addr = addr;
-  m_waitMilliSeconds = waitSeconds;
-  connect();
-}
+    : stream_(nullptr),

Review comment:
       Thanks for using the style guide naming suggestions!

##########
File path: clicache/src/CMakeLists.txt
##########
@@ -23,7 +23,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/impl/AssemblyInfo.cpp.in ${CMAKE_CURR
 list(APPEND CONFIGURE_IN_FILES ${CMAKE_CURRENT_SOURCE_DIR}/impl/AssemblyInfo.cpp.in)
 list(APPEND CONFIGURE_OUT_FILES ${CMAKE_CURRENT_BINARY_DIR}/impl/AssemblyInfo.cpp)
 
-add_library(${PROJECT_NAME} SHARED
+add_library(Apache.Geode SHARED

Review comment:
       I'm a bit confused on why `${Project_Name}` was replaced.  Looking at the commit that did the switch didn't help me much




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Link and load OpenSSL library directly
> --------------------------------------
>
>                 Key: GEODE-8102
>                 URL: https://issues.apache.org/jira/browse/GEODE-8102
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Jacob Barrett
>            Priority: Major
>              Labels: pull-request-available
>
> Lazy load the OpenSSL library directly, through ACE_SSL, into the apache-geode library. Currently we lazy load cryptoImpl, which immediately loads OpenSSL. The original intent was to avoid having an immediate dependency on OpenSSL at a time when its availability was questionable. On unix like systems OpenSSL is almost always available since so many other components in the OS depend on it. This immediate load dependency will have little to no effect on those systems. On some unix like systems the experience will improve by not having a runtime dependency on an intermediate library, cryptoImpl, that may need special treatments, like LD_LIBRARY_PATH or RPATH changes. On Windows, where OpenSSL is an anomaly we can use MSVC's lazy loading feature to only load OpenSSL if SSL/TLS is configured. This significantly improves the experience on Windows with regards to the location of cryptoImpl when using .NET.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)