szaszm commented on code in PR #2084:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2084#discussion_r2673254340
##########
extension-framework/include/utils/net/Server.h:
##########
@@ -47,9 +47,13 @@ class Server {
bool queueEmpty() {
return concurrent_queue_.empty();
}
- bool tryDequeue(utils::net::Message& received_message) {
- return concurrent_queue_.tryDequeue(received_message);
+ std::optional<utils::net::Message> tryDequeue() {
+ return concurrent_queue_.tryDequeue();
}
+ Server(const Server&) = delete;
+ Server(Server&&) = delete;
+ Server& operator=(const Server&) = delete;
+ Server& operator=(Server&&) = delete;
virtual ~Server() {
Review Comment:
rule of 5 (same with TcpClient and NetworkListenerProcessor)
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#rc-five
##########
extensions/standard-processors/processors/GetTCP.h:
##########
@@ -147,14 +147,17 @@ class GetTCP : public core::ProcessorImpl {
std::optional<size_t> max_message_size,
std::vector<utils::net::ConnectionId> connections,
std::shared_ptr<core::logging::Logger> logger);
-
+ TcpClient(const TcpClient&) = delete;
+ TcpClient(TcpClient&&) = delete;
+ TcpClient& operator=(const TcpClient&) = delete;
+ TcpClient& operator=(TcpClient&&) = delete;
~TcpClient();
void run();
void stop();
bool queueEmpty() const;
- bool tryDequeue(utils::net::Message& received_message);
+ std::optional<utils::net::Message> tryDequeue();
Review Comment:
Optional return values are cleaner than status bool return + conditionally
assigned out param, plus it allows us to remove the default empty state from
the possible states of the class, simplifying code.
##########
extension-framework/include/utils/net/Message.h:
##########
@@ -25,20 +25,21 @@
namespace org::apache::nifi::minifi::utils::net {
struct Message {
- public:
- Message() = default;
- Message(std::string message_data, IpProtocol protocol, asio::ip::address
sender_address, asio::ip::port_type server_port)
+ Message() = delete;
Review Comment:
the old default constructor left a few members uninitialized. It used to be
needed because objects were assigned from an "empty" state through out params.
Refactored those instances, so that no more default constructed "empty"
messages are necessary.
##########
libminifi/test/libtest/unit/TestUtils.h:
##########
@@ -170,11 +170,16 @@ bool countLogOccurrencesUntil(const std::string& pattern,
const size_t occurrences,
const std::chrono::milliseconds max_duration,
const std::chrono::milliseconds wait_time =
50ms);
+
+struct UdpNetworkSendResult {
+ std::error_code ec;
+ std::optional<asio::ip::udp::endpoint> local_endpoint = std::nullopt;
Review Comment:
Returning the local endpoint after a send from an unbound socket lets us
check that the proper port is set on the receiving ListenUDP end: the local
endpoint of the sending socket is the remote/sender endpoint of the listener
socket, and before/during the send, the socket is automatically bound to a
random port by the OS, just like with TCP connects.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
