szaszm commented on code in PR #1599:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1599#discussion_r1279683509
##########
libminifi/include/utils/net/Socket.h:
##########
Review Comment:
`get_last_socket_error` doesn't seem to be used outside of `Socket.cpp`
anymore. Maybe it would be best to move it to the .cpp file.
##########
libminifi/src/utils/net/DNS.cpp:
##########
Review Comment:
The winsock headers and `<netdb.h>` are no longer needed, because it's all
using asio now.
##########
libminifi/src/utils/net/DNS.cpp:
##########
@@ -127,4 +69,9 @@ nonstd::expected<std::string, std::error_code>
reverseDnsLookup(const asio::ip::
return results->host_name();
}
+std::string getMyHostName() {
+ static const std::string HOSTNAME = asio::ip::host_name();
+ return HOSTNAME;
+}
Review Comment:
Is this static caching actually needed? Is querying the hostname too
expensive?
##########
libminifi/include/utils/net/Socket.h:
##########
@@ -23,86 +23,16 @@
#endif /* WIN32_LEAN_AND_MEAN */
#include <WinSock2.h>
#else
-#include <sys/types.h>
-#include <sys/socket.h>
Review Comment:
`<sys/socket.h>` is still needed for `struct sockaddr`, but `<netdb.h>` is
no longer needed for anything.
##########
libminifi/src/utils/net/Socket.cpp:
##########
@@ -14,19 +14,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
#include "utils/net/Socket.h"
-#include "Exception.h"
-#include <cstring>
-#include <system_error>
+
#ifdef WIN32
-#ifndef WIN32_LEAN_AND_MEAN
-#define WIN32_LEAN_AND_MEAN
-#endif /* WIN32_LEAN_AND_MEAN */
Review Comment:
Why was this removed? This macro reduces the complexity of windows headers
if defined before including, and we're still including a windows header below.
##########
libminifi/src/utils/net/Socket.cpp:
##########
@@ -14,19 +14,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
#include "utils/net/Socket.h"
-#include "Exception.h"
-#include <cstring>
Review Comment:
`<cstring>` is still used for `memcpy`.
##########
libminifi/include/utils/net/DNS.h:
##########
Review Comment:
`IpProtocol.h` is no longer used. We also don't need the `struct addrinfo`
forward declaration anymore.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
