[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r377184363
##
File path: extensions/rocksdb-repos/RocksDbStream.cpp
##
@@ -28,19 +29,15 @@ namespace nifi {
namespace minifi {
namespace io {
-RocksDbStream::RocksDbStream(const std::string , rocksdb::DB *db, bool
write_enable)
+RocksDbStream::RocksDbStream(std::string path, rocksdb::DB *db, bool
write_enable)
: BaseStream(),
- path_(path),
+ path_(std::move(path)),
write_enable_(write_enable),
db_(db),
logger_(logging::LoggerFactory::getLogger()) {
rocksdb::Status status;
status = db_->Get(rocksdb::ReadOptions(), path_, _);
- if (status.ok()) {
-exists_ = true;
- } else {
-exists_ = false;
- }
+ exists_ = status.ok();
Review comment:
indeed, will remove
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r377183980
##
File path: libminifi/src/io/StreamFactory.cpp
##
@@ -38,35 +39,21 @@ namespace io {
template
class SocketCreator : public AbstractStreamFactory {
- template
Review comment:
No, I couldn't decipher the intent
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r377183616
##
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 , const
std::string , const uint16_t port, const uint16_t listeners)
-: Socket(context, hostname, port, listeners),
- ssl_(0) {
+: Socket(context, hostname, port, listeners) {
logger_ = logging::LoggerFactory::getLogger();
context_ = context;
}
TLSSocket::TLSSocket(const std::shared_ptr , const
std::string , const uint16_t port)
-: Socket(context, hostname, port, 0),
- ssl_(0) {
+: Socket(context, hostname, port, 0) {
logger_ = logging::LoggerFactory::getLogger();
context_ = context;
}
-TLSSocket::TLSSocket(const TLSSocket &)
-: Socket(std::move(d)),
- ssl_(0) {
+TLSSocket::TLSSocket(TLSSocket &) noexcept
Review comment:
I suggest resolving the code bugs in separate issues, since there are plenty.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r377182058
##
File path: libminifi/include/io/NetworkPrioritizer.h
##
@@ -83,29 +66,26 @@ class NetworkInterface {
}
}
- NetworkInterface =(const NetworkInterface &) {
-ifc_ = std::move(other.ifc_);
-prioritizer_ = std::move(other.prioritizer_);
-return *this;
- }
+ NetworkInterface =(const NetworkInterface ) = default;
+ NetworkInterface =(NetworkInterface &)
noexcept(std::is_nothrow_move_assignable::value) = default;
Review comment:
I'm OK with either keeping or removing `noexcept`, so I'll remove it. My
arguments
* For keeping: Even though string is not always marked noexcept move
assignable, it will never throw in practice and a shared_ptr move assignment
doesn't throw either
* Against keeping: string is not marked as noexcept move assignable before
C++17 so even though it will probably never throw in practice, we have no hard
guarantee.
In general about `noexcept`:
> This is very nice until someone comes along and adds another class member
whose move assignment operator throws, but forgets to update this expression
While your reasoning is correct, I believe this approach is too
conservative. We already require people to write reasonable code and depend on
certain rules being followed without compiler enforcement. Some examples that
come to my mind: destructors must not throw, classes meant for subclassing must
have a virtual destructor, every resource needs to be freed, don't return a
reference to a local or temporary.
One could make the argument that our program is only correct "until someone
comes along and" does something that violates one of these rules. Yet, we take
some risks and do not ban returning references in order to avoid accidentally
returning dangling references.
Often finding the right balance of benefits vs. risks is hard, especially
when it comes to newer features, so my strategy is to follow the style and best
practices of the C++ community. Generally people are more conservative with new
things like `noexcept`, which can sometimes be right, but it's not always the
right approach IMO.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376449122 ## File path: libminifi/include/io/ClientSocket.h ## @@ -20,10 +20,19 @@ #include +#ifdef WIN32 +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#include +#pragma comment(lib, "Ws2_32.lib") Review comment: I'll try to minimize the includes on windows and see if the `#pragma` is necessary after I've verified that my rebase doesn't break tests on Windows. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376018992
##
File path: libminifi/src/io/StreamFactory.cpp
##
@@ -76,9 +77,8 @@ class SocketCreator : public AbstractStreamFactory {
std::unique_ptr createSecureSocket(const std::string , const
uint16_t port, const std::shared_ptr
_service) {
#ifdef OPENSSL_SUPPORT
if (ssl_service != nullptr) {
- auto ptr = std::make_shared(configuration_, ssl_service);
- TLSSocket *socket = new TLSSocket(ptr, host, port);
- return std::unique_ptr(socket);
+ auto context = std::make_shared(configuration_, ssl_service);
+ return utils::make_unique(context, host, port);
} else {
Review comment:
got rid of the `else` branch and much more
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376018414
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -45,56 +42,68 @@ namespace nifi {
namespace minifi {
namespace io {
-Socket::Socket(const std::shared_ptr , const
std::string , const uint16_t port, const uint16_t listeners = -1)
-: requested_hostname_(hostname),
+static std::string get_last_err_str() noexcept {
+#ifdef WIN32
+ const auto error_code = WSAGetLastError();
+#else
+ const auto error_code = errno;
+#endif /* WIN32 */
+ return std::system_category().message(error_code);
+}
+
+Socket::Socket(const std::shared_ptr& /*context*/, std::string
hostname, const uint16_t port, const uint16_t listeners = -1)
+: addr_info_(nullptr),
+ requested_hostname_(std::move(hostname)),
+ canonical_hostname_(""),
port_(port),
- addr_info_(0),
+ is_loopback_only_(false),
socket_file_descriptor_(-1),
socket_max_(0),
total_written_(0),
total_read_(0),
- is_loopback_only_(false),
listeners_(listeners),
- canonical_hostname_(""),
nonBlocking_(false),
logger_(logging::LoggerFactory::getLogger()) {
FD_ZERO(_list_);
FD_ZERO(_fds_);
initialize_socket();
}
-Socket::Socket(const std::shared_ptr , const
std::string , const uint16_t port)
-: Socket(context, hostname, port, 0) {
- initialize_socket();
+Socket::Socket(const std::shared_ptr& context, std::string
hostname, const uint16_t port)
+: Socket(context, std::move(hostname), port, 0) {
}
-Socket::Socket(const Socket &)
-: requested_hostname_(std::move(other.requested_hostname_)),
- port_(std::move(other.port_)),
+Socket::Socket(Socket &) noexcept
+: addr_info_(other.addr_info_),
+ requested_hostname_(std::move(other.requested_hostname_)),
+ canonical_hostname_(std::move(other.canonical_hostname_)),
+ port_(other.port_),
is_loopback_only_(false),
- addr_info_(std::move(other.addr_info_)),
socket_file_descriptor_(other.socket_file_descriptor_),
- socket_max_(other.socket_max_.load()),
- listeners_(other.listeners_),
total_list_(other.total_list_),
read_fds_(other.read_fds_),
- canonical_hostname_(std::move(other.canonical_hostname_)),
+ socket_max_(other.socket_max_.load()),
+ total_written_(other.total_written_.load()),
+ total_read_(other.total_read_.load()),
+ listeners_(other.listeners_),
nonBlocking_(false),
- logger_(std::move(other.logger_)) {
- total_written_ = other.total_written_.load();
- total_read_ = other.total_read_.load();
-}
+ logger_(std::move(other.logger_))
+{ }
Socket::~Socket() {
- closeStream();
+ Socket::closeStream();
}
void Socket::closeStream() {
- if (0 != addr_info_) {
+ if (addr_info_ != nullptr) {
freeaddrinfo(addr_info_);
-addr_info_ = 0;
+addr_info_ = nullptr;
}
- if (socket_file_descriptor_ >= 0 && socket_file_descriptor_ !=
INVALID_SOCKET) {
+ if (socket_file_descriptor_ >= 0
+#ifdef WIN32
+&& socket_file_descriptor_ != INVALID_SOCKET
Review comment:
done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376018290
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -207,12 +209,31 @@ int8_t Socket::createConnection(const addrinfo *p,
in_addr_t ) {
} else {
logger_->log_error("Unknown error");
}
-
-#endif
closeStream();
socket_file_descriptor_ = -1;
return -1;
}
+#else
+ auto *sa_loc = (struct sockaddr_in*) p->ai_addr;
+ sa_loc->sin_family = AF_INET;
+ sa_loc->sin_port = htons(port_);
+ // use any address if you are connecting to the local machine for testing
+ // otherwise we must use the requested hostname
+ if (IsNullOrEmpty(requested_hostname_) || requested_hostname_ ==
"localhost") {
+if (is_loopback_only_) {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+} else {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_ANY);
+}
+ } else {
+sa_loc->sin_addr.s_addr = addr;
+ }
+ if (connect(socket_file_descriptor_, p->ai_addr, p->ai_addrlen) == -1) {
Review comment:
Changed to `< 0` since `0` is a valid file descriptor.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376017118
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -118,29 +127,26 @@ void Socket::setNonBlocking() {
nonBlocking_ = true;
}
}
-#ifdef WIN32
-int8_t Socket::createConnection(const addrinfo *p, struct in_addr ) {
-#else
-int8_t Socket::createConnection(const addrinfo *p, in_addr_t ) {
-#endif
+
+int8_t Socket::createConnection(const addrinfo *p, ip4addr ) {
if ((socket_file_descriptor_ = socket(p->ai_family, p->ai_socktype,
p->ai_protocol)) == INVALID_SOCKET) {
Review comment:
reference on the correctness of checking for `INVALID_SOCKET`/`-1`:
windows:
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-socket#return-value
posix:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/socket.html#tag_16_562_04
We define `INVALID_SOCKET` to `-1` on posix near the top of `ClientSocket.h`
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r376017118
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -118,29 +127,26 @@ void Socket::setNonBlocking() {
nonBlocking_ = true;
}
}
-#ifdef WIN32
-int8_t Socket::createConnection(const addrinfo *p, struct in_addr ) {
-#else
-int8_t Socket::createConnection(const addrinfo *p, in_addr_t ) {
-#endif
+
+int8_t Socket::createConnection(const addrinfo *p, ip4addr ) {
if ((socket_file_descriptor_ = socket(p->ai_family, p->ai_socktype,
p->ai_protocol)) == INVALID_SOCKET) {
Review comment:
reference on the correctness of checking for INVALID_SOCKET/-1:
windows:
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-socket#return-value
posix:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/socket.html#tag_16_562_04
We define `INVALID_SOCKET` to `-1` on posix near the top of ClientSocket.h
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374642621
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -207,12 +209,31 @@ int8_t Socket::createConnection(const addrinfo *p,
in_addr_t ) {
} else {
logger_->log_error("Unknown error");
}
-
-#endif
closeStream();
socket_file_descriptor_ = -1;
return -1;
}
+#else
+ auto *sa_loc = (struct sockaddr_in*) p->ai_addr;
+ sa_loc->sin_family = AF_INET;
+ sa_loc->sin_port = htons(port_);
+ // use any address if you are connecting to the local machine for testing
+ // otherwise we must use the requested hostname
+ if (IsNullOrEmpty(requested_hostname_) || requested_hostname_ ==
"localhost") {
+if (is_loopback_only_) {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+} else {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_ANY);
+}
+ } else {
+sa_loc->sin_addr.s_addr = addr;
+ }
+ if (connect(socket_file_descriptor_, p->ai_addr, p->ai_addrlen) == -1) {
Review comment:
I didn't reason about every line of touched code, although I did about some.
I wouldn't expect `connect` to ever change its behavior, but I'd like a `<= 0`
check better, too.
Noted, will fix.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374638191
##
File path: libminifi/include/utils/GeneralUtils.h
##
@@ -0,0 +1,50 @@
+/**
+ * @file GeneralUtils.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_UTILS_GENERAL_UTILS_H
+#define LIBMINIFI_INCLUDE_UTILS_GENERAL_UTILS_H
+
+#include
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+template
+std::unique_ptr make_unique(Args&&... args) {
Review comment:
It does, but neither of them are merged yet to master and both use these
utilities. Whichever is merged second will be rebased on master first and will
remove the addition.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374642621
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -207,12 +209,31 @@ int8_t Socket::createConnection(const addrinfo *p,
in_addr_t ) {
} else {
logger_->log_error("Unknown error");
}
-
-#endif
closeStream();
socket_file_descriptor_ = -1;
return -1;
}
+#else
+ auto *sa_loc = (struct sockaddr_in*) p->ai_addr;
+ sa_loc->sin_family = AF_INET;
+ sa_loc->sin_port = htons(port_);
+ // use any address if you are connecting to the local machine for testing
+ // otherwise we must use the requested hostname
+ if (IsNullOrEmpty(requested_hostname_) || requested_hostname_ ==
"localhost") {
+if (is_loopback_only_) {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+} else {
+ sa_loc->sin_addr.s_addr = htonl(INADDR_ANY);
+}
+ } else {
+sa_loc->sin_addr.s_addr = addr;
+ }
+ if (connect(socket_file_descriptor_, p->ai_addr, p->ai_addrlen) == -1) {
Review comment:
I didn't reason about every line of touched code, although I did about some.
I wouldn't expect `connect` to ever change its behavior, but I'd like a `<= 0`
check better, too.
Notes, will fix.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374641290
##
File path: libminifi/include/io/ClientSocket.h
##
@@ -93,7 +109,7 @@ class Socket : public BaseStream {
*/
virtual int16_t initialize();
- virtual void setInterface(io::NetworkInterface &) {
+ virtual void setInterface(io::NetworkInterface interface) noexcept {
Review comment:
I know `noexcept` is rarely used in our codebase but I usually prefer to
mark things that do not throw as `noexcept`. The only value is extra
documentation and one more register available during code generation, but it's
a good default IMO. The downside is if we use it carelessly, exceptions become
crashes.
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#e12-use-noexcept-when-exiting-a-function-because-of-a-throw-is-impossible-or-unacceptable
I'm open to deciding to use another approach, but in that case we need to
formalize it. I'd prefer the above.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374639255
##
File path: libminifi/src/io/ClientSocket.cpp
##
@@ -118,29 +127,26 @@ void Socket::setNonBlocking() {
nonBlocking_ = true;
}
}
-#ifdef WIN32
-int8_t Socket::createConnection(const addrinfo *p, struct in_addr ) {
-#else
-int8_t Socket::createConnection(const addrinfo *p, in_addr_t ) {
-#endif
+
+int8_t Socket::createConnection(const addrinfo *p, ip4addr ) {
if ((socket_file_descriptor_ = socket(p->ai_family, p->ai_socktype,
p->ai_protocol)) == INVALID_SOCKET) {
Review comment:
`INVALID_SOCKET` is the `-1` of windows. It only means that there was an
error, but different errors do not correspond to different return values.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
szaszm 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_r374638191
##
File path: libminifi/include/utils/GeneralUtils.h
##
@@ -0,0 +1,50 @@
+/**
+ * @file GeneralUtils.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_UTILS_GENERAL_UTILS_H
+#define LIBMINIFI_INCLUDE_UTILS_GENERAL_UTILS_H
+
+#include
+#include
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+template
+std::unique_ptr make_unique(Args&&... args) {
Review comment:
It does, but neither of them are merged yet to master and both use these
utilities.
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:
us...@infra.apache.org
With regards,
Apache Git Services
