add stringVPrintf() and stringVAppendf()

Summary:
This adds versions of stringPrintf() and stringAppendf() that accept
va_list arguments.

This also fixes the existing stringPrintf() functions to properly call
va_end() even when an exception is thrown in stringPrintfImpl().

Test Plan: Included new unit tests for stringVPrintf() and stringVAppendf().

Reviewed By: davejwatson@fb.com

Subscribers: trunkagent, doug, net-systems@, exa, njormrod

FB internal diff: D1583675

diff --git a/folly/String.cpp b/folly/String.cpp
index 4314d960b4ad95392f1fd4c7dbc0090ef425044f..9ccd09288e36159332a59f0f8fde13d62c46e8c0 100644 (file)
--- a/folly/String.cpp
+++ b/folly/String.cpp
@@ -15,7 +15,9 @@
  */
 
 #include <folly/String.h>
+
 #include <folly/Format.h>
+#include <folly/ScopeGuard.h>
 
 #include <cerrno>
 #include <cstdarg>
@@ -73,6 +75,15 @@ inline void stringPrintfImpl(std::string& output, const char* format,
 }  // anon namespace
 
 std::string stringPrintf(const char* format, ...) {
+  va_list ap;
+  va_start(ap, format);
+  SCOPE_EXIT {
+    va_end(ap);
+  };
+  return stringVPrintf(format, ap);
+}
+
+std::string stringVPrintf(const char* format, va_list ap) {
   // snprintf will tell us how large the output buffer should be, but
   // we then have to call it a second time, which is costly.  By
   // guestimating the final size, we avoid the double snprintf in many
@@ -85,10 +96,7 @@ std::string stringPrintf(const char* format, ...) {
   std::string ret(std::max(32UL, strlen(format) * 2), '\0');
   ret.resize(0);
 
-  va_list ap;
-  va_start(ap, format);
   stringPrintfImpl(ret, format, ap);
-  va_end(ap);
   return ret;
 }
 
@@ -97,17 +105,31 @@ std::string stringPrintf(const char* format, ...) {
 std::string& stringAppendf(std::string* output, const char* format, ...) {
   va_list ap;
   va_start(ap, format);
+  SCOPE_EXIT {
+    va_end(ap);
+  };
+  return stringVAppendf(output, format, ap);
+}
+
+std::string& stringVAppendf(std::string* output,
+                            const char* format,
+                            va_list ap) {
   stringPrintfImpl(*output, format, ap);
-  va_end(ap);
   return *output;
 }
 
 void stringPrintf(std::string* output, const char* format, ...) {
-  output->clear();
   va_list ap;
   va_start(ap, format);
+  SCOPE_EXIT {
+    va_end(ap);
+  };
+  return stringVPrintf(output, format, ap);
+}
+
+void stringVPrintf(std::string* output, const char* format, va_list ap) {
+  output->clear();
   stringPrintfImpl(*output, format, ap);
-  va_end(ap);
 };
 
 namespace {

diff --git a/folly/String.h b/folly/String.h
index 14fda430918e3af6314c5ddeeb6071f342dc1b09..efbda2f1bccb5b59db65082dacdc3046bdd63b3b 100644 (file)
--- a/folly/String.h
+++ b/folly/String.h
@@ -186,6 +186,16 @@ std::string& stringAppendf(std::string* output,
                           FOLLY_PRINTF_FORMAT const char* format, ...)
   FOLLY_PRINTF_FORMAT_ATTR(2, 3);
 
+/**
+ * Similar to stringPrintf, but accepts a va_list argument.
+ *
+ * As with vsnprintf() itself, the value of ap is undefined after the call.
+ * These functions do not call va_end() on ap.
+ */
+std::string stringVPrintf(const char* format, va_list ap);
+void stringVPrintf(std::string* out, const char* format, va_list ap);
+std::string& stringVAppendf(std::string* out, const char* format, va_list ap);
+
 /**
  * Backslashify a string, that is, replace non-printable characters
  * with C-style (but NOT C compliant) "\xHH" encoding.  If hex_style

diff --git a/folly/test/StringTest.cpp b/folly/test/StringTest.cpp
index f2162567dd8830b2e8d91d7545653e95dbfc0f3c..e24f30285302dd9dc7735d4afac3bce8c8c75209 100644 (file)
--- a/folly/test/StringTest.cpp
+++ b/folly/test/StringTest.cpp
@@ -16,6 +16,7 @@
 
 #include <folly/String.h>
 
+#include <cstdarg>
 #include <random>
 #include <boost/algorithm/string.hpp>
 #include <gtest/gtest.h>
@@ -60,6 +61,56 @@ TEST(StringPrintf, Appending) {
   EXPECT_EQ(s, "abc 123");
 }
 
+void vprintfCheck(const char* expected, const char* fmt, ...) {
+  va_list apOrig;
+  va_start(apOrig, fmt);
+  SCOPE_EXIT {
+    va_end(apOrig);
+  };
+  va_list ap;
+  va_copy(ap, apOrig);
+  SCOPE_EXIT {
+    va_end(ap);
+  };
+
+  // Check both APIs for calling stringVPrintf()
+  EXPECT_EQ(expected, stringVPrintf(fmt, ap));
+  va_end(ap);
+  va_copy(ap, apOrig);
+
+  std::string out;
+  stringVPrintf(&out, fmt, ap);
+  va_end(ap);
+  va_copy(ap, apOrig);
+  EXPECT_EQ(expected, out);
+
+  // Check stringVAppendf() as well
+  std::string prefix = "foobar";
+  out = prefix;
+  EXPECT_EQ(prefix + expected, stringVAppendf(&out, fmt, ap));
+  va_end(ap);
+  va_copy(ap, apOrig);
+}
+
+void vprintfError(const char* fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  SCOPE_EXIT {
+    va_end(ap);
+  };
+
+  EXPECT_THROW({stringVPrintf(fmt, ap);},
+               std::runtime_error);
+}
+
+TEST(StringPrintf, VPrintf) {
+  vprintfCheck("foo", "%s", "foo");
+  vprintfCheck("long string requiring reallocation 1 2 3 0x12345678",
+               "%s %s %d %d %d %#x",
+               "long string", "requiring reallocation", 1, 2, 3, 0x12345678);
+  vprintfError("bogus%", "foo");
+}
+
 TEST(StringPrintf, VariousSizes) {
   // Test a wide variety of output sizes
   for (int i = 0; i < 100; ++i) {