From a0b1b15d59631162af3755883b629f37f0cb3094 Mon Sep 17 00:00:00 2001 From: Guohao Li Date: Thu, 25 Jan 2024 14:54:51 +0800 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Dan Wang --- src/runtime/rpc/rpc_address.cpp | 46 +++++++++++++++++++------------ src/runtime/rpc/rpc_address.h | 7 +++++ src/runtime/rpc/rpc_host_port.cpp | 33 ++-------------------- 3 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/runtime/rpc/rpc_address.cpp b/src/runtime/rpc/rpc_address.cpp index 0f3ed0986c..c8dba854e6 100644 --- a/src/runtime/rpc/rpc_address.cpp +++ b/src/runtime/rpc/rpc_address.cpp @@ -27,9 +27,7 @@ #include "runtime/rpc/rpc_address.h" #include -#include #include -#include #include #include #include @@ -39,11 +37,32 @@ #include "utils/fixed_size_buffer_pool.h" #include "utils/fmt_logging.h" #include "utils/ports.h" -#include "utils/safe_strerror_posix.h" #include "utils/string_conv.h" #include "utils/strings.h" namespace dsn { +/*static*/ +error_s rpc_address::GetAddrInfo(const std::string &hostname, const addrinfo &hints, AddrInfo *info) +{ + addrinfo *res = nullptr; + const int rc = getaddrinfo(hostname.c_str(), nullptr, &hints, &res); + const int err = errno; // preserving the errno from the getaddrinfo() call + AddrInfo result(res, ::freeaddrinfo); + if (rc != 0) { + if (rc == EAI_SYSTEM) { + LOG_ERROR( + "getaddrinfo failed, name = {}, err = {}", hostname, utils::safe_strerror(rc)); + return error_s::make(ERR_NETWORK_FAILURE, utils::safe_strerror(err)); + } + LOG_ERROR("getaddrinfo failed, name = {}, err = {}", hostname, gai_strerror(rc)); + return error_s::make(ERR_NETWORK_FAILURE, gai_strerror(rc)); + } + + if (info != nullptr) { + info->swap(result); + } + return error_s::ok(); +} const rpc_address rpc_address::s_invalid_address; @@ -54,23 +73,16 @@ uint32_t rpc_address::ipv4_from_host(const char *name) memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_INET; hints.ai_socktype = SOCK_STREAM; - addrinfo *res = nullptr; - - const int rc = getaddrinfo(name, nullptr, &hints, &res); - const int err = errno; // preserving the errno from the getaddrinfo() call - - if (rc != 0) { - if (rc == EAI_SYSTEM) { - LOG_ERROR("getaddrinfo failed, name = {}, err = {}", name, utils::safe_strerror(err)); - } - LOG_ERROR("getaddrinfo failed, name = {}, err = {}", name, gai_strerror(err)); + AddrInfo result; + if (dsn_unlikely(!GetAddrInfo(name, hints, &result).is_ok())) { return 0; } - - CHECK(res->ai_family == AF_INET, ""); - struct sockaddr_in *ipv4 = (struct sockaddr_in *)res->ai_addr; + CHECK_EQ(result.get()->ai_family, AF_INET); + auto *ipv4 = reinterpret_cast(result.get()->ai_addr); // converts from network byte order to host byte order - return (uint32_t)ntohl(ipv4->sin_addr.s_addr); + uint32_t iphost = ntohl(ipv4->sin_addr.s_addr); + freeaddrinfo(result.get()); + return iphost; } /*static*/ diff --git a/src/runtime/rpc/rpc_address.h b/src/runtime/rpc/rpc_address.h index e5afd7270a..bec19e80d4 100644 --- a/src/runtime/rpc/rpc_address.h +++ b/src/runtime/rpc/rpc_address.h @@ -27,6 +27,8 @@ #pragma once #include // IWYU pragma: keep +#include +#include #include #include // IWYU pragma: no_include @@ -35,6 +37,8 @@ #include #include +#include "utils/errors.h" +#include "utils/safe_strerror_posix.h" #include "utils/fmt_utils.h" namespace apache { @@ -54,6 +58,8 @@ USER_DEFINED_ENUM_FORMATTER(dsn_host_type_t) namespace dsn { +using AddrInfo = std::unique_ptr>; + class rpc_group_address; class rpc_address @@ -64,6 +70,7 @@ class rpc_address static bool is_site_local_address(uint32_t ip_net); static uint32_t ipv4_from_host(const char *hostname); static uint32_t ipv4_from_network_interface(const char *network_interface); + static error_s GetAddrInfo(const std::string &hostname, const addrinfo &hints, AddrInfo *info); ~rpc_address(); diff --git a/src/runtime/rpc/rpc_host_port.cpp b/src/runtime/rpc/rpc_host_port.cpp index 2da05e7b8f..ad9b64029b 100644 --- a/src/runtime/rpc/rpc_host_port.cpp +++ b/src/runtime/rpc/rpc_host_port.cpp @@ -39,30 +39,6 @@ namespace dsn { const host_port host_port::s_invalid_host_port; -namespace { - -using AddrInfo = std::unique_ptr>; - -error_s GetAddrInfo(const std::string &hostname, const addrinfo &hints, AddrInfo *info) -{ - addrinfo *res = nullptr; - const int rc = getaddrinfo(hostname.c_str(), nullptr, &hints, &res); - const int err = errno; // preserving the errno from the getaddrinfo() call - AddrInfo result(res, ::freeaddrinfo); - if (rc != 0) { - if (rc == EAI_SYSTEM) { - return error_s::make(ERR_NETWORK_FAILURE, utils::safe_strerror(err)); - } - return error_s::make(ERR_NETWORK_FAILURE, gai_strerror(rc)); - } - - if (info != nullptr) { - info->swap(result); - } - return error_s::ok(); -} -} - host_port::host_port(std::string host, uint16_t port) : _host(std::move(host)), _port(port), _type(HOST_TYPE_IPV4) { @@ -100,11 +76,7 @@ bool host_port::from_string(const std::string &s) return false; } - struct addrinfo hints; - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_INET; - hints.ai_socktype = SOCK_STREAM; - if (dsn_unlikely(!GetAddrInfo(_host, hints, nullptr))) { + if (dsn_unlikely(rpc_address::ipv4_from_host(_host.c_str()) == 0)) { return false; } @@ -194,7 +166,7 @@ error_s host_port::resolve_addresses(std::vector &addresses) const hints.ai_family = AF_INET; hints.ai_socktype = SOCK_STREAM; AddrInfo result; - RETURN_NOT_OK(GetAddrInfo(_host, hints, &result)); + RETURN_NOT_OK(rpc_address::GetAddrInfo(_host, hints, &result)); // DNS may return the same host multiple times. We want to return only the unique // addresses, but in the same order as DNS returned them. To do so, we keep track @@ -212,6 +184,7 @@ error_s host_port::resolve_addresses(std::vector &addresses) const } } + freeaddrinfo(result.get()); if (result_addresses.empty()) { return error_s::make(dsn::ERR_NETWORK_FAILURE, fmt::format("can not resolve host_port {}.", to_string()));