refactor(host_port): Unify the hostname lookup function

src/base/pegasus_utils.cpp

@@ -19,35 +19,14 @@
 
 #include "pegasus_utils.h"
 
-#include <arpa/inet.h>
-#include <netdb.h>
-#include <netinet/in.h>
 #include <stdio.h>
-#include <sys/socket.h>
 #include <cctype>
 
-#include "runtime/rpc/rpc_address.h"
 #include "utils/fmt_logging.h"
 
 namespace pegasus {
 namespace utils {
 
-void addr2host(const ::dsn::rpc_address &addr, char *str, int len /* = 100*/)
-{
-    struct sockaddr_in addr2;
-    addr2.sin_addr.s_addr = htonl(addr.ip());
-    addr2.sin_family = AF_INET;
-    if (getnameinfo((struct sockaddr *)&addr2,
-                    sizeof(sockaddr),
-                    str,
-                    sizeof(char *) * len,
-                    nullptr,
-                    0,
-                    NI_NAMEREQD)) {
-        inet_ntop(AF_INET, &(addr2.sin_addr), str, 100);
-    }
-}
-
 size_t
 c_escape_string(const char *src, size_t src_len, char *dest, size_t dest_len, bool always_escape)
 {

src/base/pegasus_utils.h

@@ -28,8 +28,8 @@
 #include <string>
 #include <vector>
 
-#include "utils/flags.h"
 #include "absl/strings/string_view.h"
+#include "utils/flags.h"
 
 DSN_DECLARE_bool(encrypt_data_at_rest);
 
@@ -45,9 +45,6 @@ const uint32_t epoch_begin = 1451606400;
 inline uint32_t epoch_now() { return time(nullptr) - epoch_begin; }
 const static std::string kRedactedString = "<redacted>";
 
-// extract "host" from rpc_address
-void addr2host(const ::dsn::rpc_address &addr, char *str, int len);
-
 template <typename elem_type, typename compare = std::less<elem_type>>
 class top_n
 {

src/runtime/rpc/rpc_host_port.cpp

@@ -18,6 +18,7 @@
  */
 
 #include <arpa/inet.h>
+#include <errno.h>
 #include <netdb.h>
 #include <netinet/in.h>
 #include <sys/socket.h>
@@ -32,9 +33,9 @@
 #include "utils/api_utilities.h"
 #include "utils/error_code.h"
 #include "utils/ports.h"
+#include "utils/safe_strerror_posix.h"
 #include "utils/string_conv.h"
 #include "utils/timer.h"
-#include "utils/utils.h"
 
 namespace dsn {
 
@@ -57,9 +58,9 @@ host_port host_port::from_address(rpc_address addr)
         WARNING, 100, "construct host_port '{}' from rpc_address '{}'", hp, addr);
     switch (addr.type()) {
     case HOST_TYPE_IPV4: {
-        CHECK(utils::hostname_from_ip(htonl(addr.ip()), &hp._host),
-              "invalid host_port {}",
-              addr.ipv4_str());
+        CHECK_OK(lookup_hostname(htonl(addr.ip()), &hp._host),
+                 "lookup_hostname failed for {}",
+                 addr.ipv4_str());
         hp._port = addr.port();
     } break;
     case HOST_TYPE_GROUP: {
@@ -216,4 +217,34 @@ error_s host_port::resolve_addresses(std::vector<rpc_address> &addresses) const
     return error_s::ok();
 }
 
+error_s host_port::lookup_hostname(uint32_t ip, std::string *hostname)
+{
+    struct sockaddr_in addr_in;
+    addr_in.sin_family = AF_INET;
+    addr_in.sin_port = 0;
+    addr_in.sin_addr.s_addr = ip;
+    char host[NI_MAXHOST];
+    int rc = ::getnameinfo((struct sockaddr *)(&addr_in),
+                           sizeof(struct sockaddr),
+                           host,
+                           sizeof(host),
+                           nullptr,
+                           0,
+                           NI_NAMEREQD);
+    if (dsn_unlikely(rc != 0)) {
+        if (rc == EAI_SYSTEM) {
+            return error_s::make(dsn::ERR_NETWORK_FAILURE,
+                                 fmt::format("{}: {}: getnameinfo failed",
+                                             gai_strerror(rc),
+                                             dsn::utils::safe_strerror(errno)));
+        }
+
+        return error_s::make(dsn::ERR_NETWORK_FAILURE,
+                             fmt::format("{}: getnameinfo failed", gai_strerror(rc)));
+    }
+
+    *hostname = host;
+    return error_s::ok();
+}
+
 } // namespace dsn

src/runtime/rpc/rpc_host_port.h

@@ -313,6 +313,7 @@ class host_port
 private:
     friend class dns_resolver;
     friend class rpc_group_host_port;
+    FRIEND_TEST(host_port_test, lookup_hostname);
     FRIEND_TEST(host_port_test, transfer_rpc_address);
 
     static const host_port s_invalid_host_port;
@@ -321,6 +322,9 @@ class host_port
     // There may be multiple rpc_addresses for one host_port.
     error_s resolve_addresses(std::vector<rpc_address> &addresses) const;
 
+    // Does reverse DNS lookup of the address and stores it in hostname.
+    static error_s lookup_hostname(uint32_t ip, std::string *hostname);
+
     std::string _host;
     uint16_t _port = 0;
     dsn_host_type_t _type = HOST_TYPE_INVALID;

src/runtime/test/host_port_test.cpp

@@ -18,6 +18,7 @@
  */
 
 #include <fmt/core.h>
+#include <netinet/in.h>
 #include <map>
 #include <memory>
 #include <string>
@@ -262,6 +263,21 @@ TEST(host_port_test, thrift_parser)
     send_and_check_host_port_by_serialize(hp2, DSF_THRIFT_JSON);
 }
 
+TEST(host_port_test, lookup_hostname)
+{
+    const std::string valid_ip = "127.0.0.1";
+    const std::string expected_hostname = "localhost";
+
+    const auto rpc_example_valid = rpc_address::from_ip_port(valid_ip, 23010);
+    std::string hostname;
+    auto es = host_port::lookup_hostname(::htonl(rpc_example_valid.ip()), &hostname);
+    ASSERT_TRUE(es.is_ok()) << es.description();
+    ASSERT_EQ(expected_hostname, hostname);
+
+    es = host_port::lookup_hostname(12321, &hostname);
+    ASSERT_FALSE(es.is_ok());
+}
+
 TEST(host_port_test, test_macros)
 {
     static const host_port kHp1("localhost", 8081);

src/utils/utils.h

@@ -67,21 +67,6 @@ std::shared_ptr<T> make_shared_array(size_t size)
     return std::shared_ptr<T>(new T[size], std::default_delete<T[]>());
 }
 
-// get host name from ip series
-// if can't get a hostname from ip(maybe no hostname or other errors), return false, and
-// hostname_result will be invalid value
-// if multiple hostname got and all of them are resolvable return true, otherwise return false.
-// and the hostname_result will be "hostname1,hostname2(or ip_address or )..."
-// we only support ipv4 currently
-// check if a.b.c.d:port can be resolved to hostname:port. If it can be resolved, return true
-// and hostname_result
-// will be the hostname, or it will be ip address or error message
-
-// TODO(yingchun): Consider to move it to rpc_address.
-// valid_ip_network_order -> return TRUE && hostname_result=hostname	|
-// invalid_ip_network_order -> return FALSE
-bool hostname_from_ip(uint32_t ip, std::string *hostname_result);
-
 template <typename A, typename B>
 std::multimap<B, A> flip_map(const std::map<A, B> &source)
 {