Eliminate a string allocation in folly::SocketAddr and bug in char[] conversion

Summary:
We were doing an unnecessary and wasteful string conversion in the
SocketAddr codepath.  This eliminates it.  I also noticed we had an off-by-one
in the "convert string to char buffer" code path, so I added a test to confirm
the bug and fixed it.

Reviewed By: yfeldblum, meyering

Differential Revision: D3817959

fbshipit-source-id: 51fed8331ab23c0888a3d1f9e0cc9cea5ea8329b

diff --git a/folly/SocketAddress.cpp b/folly/SocketAddress.cpp
index bdafdb02bceaff1615dea9d78ec7bf376a23cf85..cd1ebdf8c69bfb4b9d406e6063f230d78610cd86 100644 (file)
--- a/folly/SocketAddress.cpp
+++ b/folly/SocketAddress.cpp
@@ -373,18 +373,16 @@ std::string SocketAddress::getFullyQualified() const {
 }
 
 std::string SocketAddress::getAddressStr() const {
-  char buf[INET6_ADDRSTRLEN];
-  getAddressStr(buf, sizeof(buf));
-  return buf;
-}
-
-void SocketAddress::getAddressStr(char* buf, size_t buflen) const {
   auto family = getFamily();
   if (family != AF_INET && family != AF_INET6) {
     throw std::invalid_argument("Can't get address str for non ip address");
   }
-  std::string ret = storage_.addr.str();
-  size_t len = std::min(buflen, ret.size());
+  return storage_.addr.str();
+}
+
+void SocketAddress::getAddressStr(char* buf, size_t buflen) const {
+  auto ret = getAddressStr();
+  size_t len = std::min(buflen - 1, ret.size());
   memcpy(buf, ret.data(), len);
   buf[len] = '\0';
 }

diff --git a/folly/test/SocketAddressTest.cpp b/folly/test/SocketAddressTest.cpp
index c04d9bfcc5ae8b0cdc088b2fcdf386a14f24dfcc..145aa863b2c365696b035e72d0c973848457f20d 100644 (file)
--- a/folly/test/SocketAddressTest.cpp
+++ b/folly/test/SocketAddressTest.cpp
@@ -51,6 +51,15 @@ TEST(SocketAddress, ConstructFromIpv4) {
   EXPECT_EQ(inaddr->sin_port, htons(4321));
 }
 
+TEST(SocketAddress, StringConversion) {
+  SocketAddress addr("1.2.3.4", 4321);
+  EXPECT_EQ(addr.getFamily(), AF_INET);
+  EXPECT_EQ(addr.getAddressStr(), "1.2.3.4");
+  char buf[30];
+  addr.getAddressStr(buf, 2);
+  EXPECT_STREQ(buf, "1");
+}
+
 TEST(SocketAddress, IPv4ToStringConversion) {
   // testing addresses *.5.5.5, 5.*.5.5, 5.5.*.5, 5.5.5.*
   SocketAddress addr;