[PDNCF] Make AddrInfoGetter::getaddrinfo return a unique_ptr

Return a unique_ptr with a custom deleter from
net::AddrInfoGetter::getaddrinfo.
This makes memory management more
consistent with C++ style and less error-prone.

Checkout :
https://chromium-review.googlesource.com/c/chromium/src/+/3495725

Change-Id: I57454792e2ca9d363b28384eb63d6e7c92f33307
Signed-off-by: feifei08.liu <feifei08.liu@samsung.com>

diff --git a/net/dns/address_info.cc b/net/dns/address_info.cc
index ce412d2012dc839942d1e58b9e8172d39555b37b..730f2a5fe16224b9ae720a1b890f81f6beef818f 100644 (file)
--- a/net/dns/address_info.cc
+++ b/net/dns/address_info.cc
@@ -4,6 +4,8 @@
 
 #include "net/dns/address_info.h"
 
+#include <memory>
+
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "base/sys_byteorder.h"
@@ -53,7 +55,8 @@ AddressInfo::AddressInfoAndResult AddressInfo::Get(
     getter = std::make_unique<AddrInfoGetter>();
   int err = OK;
   int os_error = 0;
-  addrinfo* ai = getter->getaddrinfo(host, &hints, &os_error);
+  std::unique_ptr<addrinfo, FreeAddrInfoFunc> ai =
+      getter->getaddrinfo(host, &hints, &os_error);
 
   if (!ai) {
     err = ERR_NAME_NOT_RESOLVED;
@@ -76,31 +79,21 @@ AddressInfo::AddressInfoAndResult AddressInfo::Get(
     return AddressInfoAndResult(absl::optional<AddressInfo>(), err, os_error);
   }
 
-  return AddressInfoAndResult(
-      absl::optional<AddressInfo>(AddressInfo(ai, std::move(getter))), OK, 0);
+  return AddressInfoAndResult(absl::optional<AddressInfo>(AddressInfo(
+                                  std::move(ai), std::move(getter))),
+                              OK, 0);
 }
 
-AddressInfo::AddressInfo(AddressInfo&& other)
-    : ai_(other.ai_), getter_(std::move(other.getter_)) {
-  other.ai_ = nullptr;
-}
+AddressInfo::AddressInfo(AddressInfo&& other) = default;
 
-AddressInfo& AddressInfo::operator=(AddressInfo&& other) {
-  ai_ = other.ai_;
-  other.ai_ = nullptr;
-  getter_ = std::move(other.getter_);
-  return *this;
-}
+AddressInfo& AddressInfo::operator=(AddressInfo&& other) = default;
 
-AddressInfo::~AddressInfo() {
-  if (ai_)
-    getter_->freeaddrinfo(ai_);
-}
+AddressInfo::~AddressInfo() = default;
 
 //// public methods
 
 AddressInfo::const_iterator AddressInfo::begin() const {
-  return const_iterator(ai_);
+  return const_iterator(ai_.get());
 }
 
 AddressInfo::const_iterator AddressInfo::end() const {
@@ -116,7 +109,7 @@ absl::optional<std::string> AddressInfo::GetCanonicalName() const {
 bool AddressInfo::IsAllLocalhostOfOneFamily() const {
   bool saw_v4_localhost = false;
   bool saw_v6_localhost = false;
-  const auto* ai = ai_;
+  const auto* ai = ai_.get();
   for (; ai != nullptr; ai = Next(ai)) {
     switch (ai->ai_family) {
       case AF_INET: {
@@ -167,32 +160,39 @@ AddressList AddressInfo::CreateAddressList() const {
 
 //// private methods
 
-AddressInfo::AddressInfo(addrinfo* ai, std::unique_ptr<AddrInfoGetter> getter)
-    : ai_(ai), getter_(std::move(getter)) {}
+AddressInfo::AddressInfo(std::unique_ptr<addrinfo, FreeAddrInfoFunc> ai,
+                         std::unique_ptr<AddrInfoGetter> getter)
+    : ai_(std::move(ai)), getter_(std::move(getter)) {}
+
 
 //// AddrInfoGetter
 
 AddrInfoGetter::AddrInfoGetter() = default;
 AddrInfoGetter::~AddrInfoGetter() = default;
 
-addrinfo* AddrInfoGetter::getaddrinfo(const std::string& host,
+std::unique_ptr<addrinfo, FreeAddrInfoFunc> AddrInfoGetter::getaddrinfo(
+                                      const std::string& host,
                                       const addrinfo* hints,
                                       int* out_os_error) {
   addrinfo* ai;
+  // We wrap freeaddrinfo() in a lambda just in case some operating systems use
+  // a different signature for it.
+  FreeAddrInfoFunc deleter = [](addrinfo* ai) { ::freeaddrinfo(ai); };
+
+  std::unique_ptr<addrinfo, FreeAddrInfoFunc> rv = {nullptr, deleter};
+
   *out_os_error = ::getaddrinfo(host.c_str(), nullptr, hints, &ai);
 
   if (*out_os_error) {
 #if defined(OS_WIN)
     *out_os_error = WSAGetLastError();
 #endif
-    return nullptr;
+    return rv;
   }
 
-  return ai;
-}
+  rv.reset(ai);
+  return rv;
 
-void AddrInfoGetter::freeaddrinfo(addrinfo* ai) {
-  ::freeaddrinfo(ai);
 }
 
 }  // namespace net

diff --git a/net/dns/address_info.h b/net/dns/address_info.h
index 93de5725cf1120d66465aa171548d5f43c009f68..eb25dacb618b55ee2e9aca5019733a5cee8551c2 100644 (file)
--- a/net/dns/address_info.h
+++ b/net/dns/address_info.h
@@ -21,6 +21,8 @@ namespace net {
 class AddressList;
 class AddrInfoGetter;
 
+using FreeAddrInfoFunc = void (*)(addrinfo*);
+
 // AddressInfo -- this encapsulates the system call to getaddrinfo and the
 // data structure that it populates and returns.
 class NET_EXPORT_PRIVATE AddressInfo {
@@ -42,6 +44,7 @@ class NET_EXPORT_PRIVATE AddressInfo {
     const addrinfo& operator*() const;
 
    private:
+    // Owned by AddressInfo.
     const addrinfo* ai_;
   };
 
@@ -71,10 +74,12 @@ class NET_EXPORT_PRIVATE AddressInfo {
 
  private:
   // Constructors
-  AddressInfo(addrinfo* ai, std::unique_ptr<AddrInfoGetter> getter);
+  AddressInfo(std::unique_ptr<addrinfo, FreeAddrInfoFunc> ai,
+              std::unique_ptr<AddrInfoGetter> getter);
 
   // Data.
-  addrinfo* ai_;  // Never null (except after move)
+  std::unique_ptr<addrinfo, FreeAddrInfoFunc>
+      ai_;  // Never null (except after move)
   std::unique_ptr<AddrInfoGetter> getter_;
 
   DISALLOW_COPY_AND_ASSIGN(AddressInfo);
@@ -86,10 +91,10 @@ class NET_EXPORT_PRIVATE AddrInfoGetter {
   AddrInfoGetter();
   // Virtual for tests.
   virtual ~AddrInfoGetter();
-  virtual addrinfo* getaddrinfo(const std::string& host,
-                                const addrinfo* hints,
-                                int* out_os_error);
-  virtual void freeaddrinfo(addrinfo* ai);
+  virtual std::unique_ptr<addrinfo, FreeAddrInfoFunc> getaddrinfo(
+      const std::string& host,
+      const addrinfo* hints,
+      int* out_os_error);
 
   DISALLOW_COPY_AND_ASSIGN(AddrInfoGetter);
 };