nifi-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
Date Mon, 10 Feb 2020 17:25:12 GMT
bakaid commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets +
clean up issues
URL: https://github.com/apache/nifi-minifi-cpp/pull/713#discussion_r377205834
 
 

 ##########
 File path: libminifi/src/io/tls/TLSSocket.cpp
 ##########
 @@ -170,22 +174,19 @@ void TLSSocket::closeStream() {
  * @param listeners number of listeners in the queue
  */
 TLSSocket::TLSSocket(const std::shared_ptr<TLSContext> &context, const std::string
&hostname, const uint16_t port, const uint16_t listeners)
-    : Socket(context, hostname, port, listeners),
-      ssl_(0) {
+    : Socket(context, hostname, port, listeners) {
   logger_ = logging::LoggerFactory<TLSSocket>::getLogger();
   context_ = context;
 }
 
 TLSSocket::TLSSocket(const std::shared_ptr<TLSContext> &context, const std::string
&hostname, const uint16_t port)
-    : Socket(context, hostname, port, 0),
-      ssl_(0) {
+    : Socket(context, hostname, port, 0) {
   logger_ = logging::LoggerFactory<TLSSocket>::getLogger();
   context_ = context;
 }
 
-TLSSocket::TLSSocket(const TLSSocket &&d)
-    : Socket(std::move(d)),
-      ssl_(0) {
+TLSSocket::TLSSocket(TLSSocket &&d) noexcept
 
 Review comment:
   Since this is a substantial change to the sockets, I would like to run extensive tests
on them on multiple platforms, which I would not like to do on code we know is faulty. This
is exactly as bad as it was, but the Socket's move constructor has been changed, but it is
still bad.
   I am OK with a new issue or separate commits in this PR, altough I don't see the point,
I think it fits in the scope of the current issue very well.
   I am -1 on this PR until these are fixed.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

Mime
View raw message