SkStringPrintf and SkString::printf now are no longer limted by a static buffer

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1403803002

Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c

Review URL: https://codereview.chromium.org/1403803002

3 files changed, 85 insertions, 61 deletions

diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 24b1b8fb62..528c602abb 100644
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -33,6 +33,58 @@ static const size_t kBufferSize = 1024;
         va_end(args);                                      \
     } while (0)
 
+#ifdef SK_BUILD_FOR_WIN
+#define V_SKSTRING_PRINTF(output, format)                               \
+    do {                                                                \
+        va_list args;                                                   \
+        va_start(args, format);                                         \
+        char buffer[kBufferSize];                                       \
+        int length = _vsnprintf_s(buffer, sizeof(buffer),               \
+                                  _TRUNCATE, format, args);             \
+        va_end(args);                                                   \
+        if (length >= 0 && length < (int)sizeof(buffer)) {              \
+            output.set(buffer, length);                                 \
+            break;                                                      \
+        }                                                               \
+        va_start(args, format);                                         \
+        length = _vscprintf(format, args);                              \
+        va_end(args);                                                   \
+        SkAutoTMalloc<char> autoTMalloc((size_t)length + 1);            \
+        va_start(args, format);                                         \
+        SkDEBUGCODE(int check = ) _vsnprintf_s(autoTMalloc.get(),       \
+                                               length + 1, _TRUNCATE,   \
+                                               format, args);           \
+        va_end(args);                                                   \
+        SkASSERT(check == length);                                      \
+        output.set(autoTMalloc.get(), length);                          \
+        SkASSERT(output[length] == '\0');                               \
+    } while (false)
+#else
+#define V_SKSTRING_PRINTF(output, format)                               \
+    do {                                                                \
+        va_list args;                                                   \
+        va_start(args, format);                                         \
+        char buffer[kBufferSize];                                       \
+        int length = vsnprintf(buffer, sizeof(buffer), format, args);   \
+        va_end(args);                                                   \
+        if (length < 0) {                                               \
+            break;                                                      \
+        }                                                               \
+        if (length < (int)sizeof(buffer)) {                             \
+            output.set(buffer, length);                                 \
+            break;                                                      \
+        }                                                               \
+        SkAutoTMalloc<char> autoTMalloc((size_t)length + 1);            \
+        va_start(args, format);                                         \
+        SkDEBUGCODE(int check = ) vsnprintf(autoTMalloc.get(),          \
+                                            length + 1, format, args);  \
+        va_end(args);                                                   \
+        SkASSERT(check == length);                                      \
+        output.set(autoTMalloc.get(), length);                          \
+        SkASSERT(output[length] == '\0');                               \
+    } while (false)
+#endif
+
 ///////////////////////////////////////////////////////////////////////////////
 
 bool SkStrEndsWith(const char string[], const char suffixStr[]) {
@@ -513,11 +565,7 @@ void SkString::insertScalar(size_t offset, SkScalar value) {
 }
 
 void SkString::printf(const char format[], ...) {
-    char    buffer[kBufferSize];
-    int length;
-    ARGS_TO_BUFFER(format, buffer, kBufferSize, length);
-
-    this->set(buffer, length);
+    V_SKSTRING_PRINTF((*this), format);
 }
 
 void SkString::appendf(const char format[], ...) {
@@ -593,10 +641,7 @@ void SkString::swap(SkString& other) {
 
 SkString SkStringPrintf(const char* format, ...) {
     SkString formattedOutput;
-    char buffer[kBufferSize];
-    SK_UNUSED int length;
-    ARGS_TO_BUFFER(format, buffer, kBufferSize, length);
-    formattedOutput.set(buffer);
+    V_SKSTRING_PRINTF(formattedOutput, format);
     return formattedOutput;
 }
 
diff --git a/src/pdf/SkPDFMetadata.cpp b/src/pdf/SkPDFMetadata.cpp
index 2765d4d189..d3abd78312 100644
--- a/src/pdf/SkPDFMetadata.cpp
+++ b/src/pdf/SkPDFMetadata.cpp
@@ -95,54 +95,6 @@ SkPDFObject* SkPDFMetadata::CreatePdfId(const UUID& doc, const UUID& instance) {
     return array.release();
 }
 
-// Improvement on SkStringPrintf to allow for arbitrarily long output.
-// TODO: replace SkStringPrintf.
-static SkString sk_string_printf(const char* format, ...) {
-#ifdef SK_BUILD_FOR_WIN
-    va_list args;
-    va_start(args, format);
-    char buffer[1024];
-    int length = _vsnprintf_s(buffer, sizeof(buffer), _TRUNCATE, format, args);
-    va_end(args);
-    if (length >= 0 && length < (int)sizeof(buffer)) {
-        return SkString(buffer, length);
-    }
-    va_start(args, format);
-    length = _vscprintf(format, args);
-    va_end(args);
-
-    SkString string((size_t)length);
-    va_start(args, format);
-    SkDEBUGCODE(int check = ) _vsnprintf_s(string.writable_str(), length + 1,
-                                           _TRUNCATE, format, args);
-    va_end(args);
-    SkASSERT(check == length);
-    SkASSERT(string[length] == '\0');
-    return string;
-#else  // C99/C++11 standard vsnprintf
-    // TODO: When all compilers support this, remove windows-specific code.
-    va_list args;
-    va_start(args, format);
-    char buffer[1024];
-    int length = vsnprintf(buffer, sizeof(buffer), format, args);
-    va_end(args);
-    if (length < 0) {
-        return SkString();
-    }
-    if (length < (int)sizeof(buffer)) {
-        return SkString(buffer, length);
-    }
-    SkString string((size_t)length);
-    va_start(args, format);
-    SkDEBUGCODE(int check = )
-            vsnprintf(string.writable_str(), length + 1, format, args);
-    va_end(args);
-    SkASSERT(check == length);
-    SkASSERT(string[length] == '\0');
-    return string;
-#endif
-}
-
 static const SkString get(const SkTArray<SkDocument::Attribute>& info,
                           const char* key) {
     for (const auto& keyValue : info) {
@@ -303,14 +255,14 @@ SkPDFObject* SkPDFMetadata::createXMPObject(const UUID& doc,
         fCreation->toISO8601(&tmp);
         SkASSERT(0 == count_xml_escape_size(tmp));
         // YYYY-mm-ddTHH:MM:SS[+|-]ZZ:ZZ; no need to escape
-        creationDate = sk_string_printf("<xmp:CreateDate>%s</xmp:CreateDate>\n",
-                                        tmp.c_str());
+        creationDate = SkStringPrintf("<xmp:CreateDate>%s</xmp:CreateDate>\n",
+                                      tmp.c_str());
     }
     if (fModified) {
         SkString tmp;
         fModified->toISO8601(&tmp);
         SkASSERT(0 == count_xml_escape_size(tmp));
-        modificationDate = sk_string_printf(
+        modificationDate = SkStringPrintf(
                 "<xmp:ModifyDate>%s</xmp:ModifyDate>\n", tmp.c_str());
     }
     SkString title = escape_xml(
@@ -339,7 +291,7 @@ SkPDFObject* SkPDFMetadata::createXMPObject(const UUID& doc,
     SkASSERT(0 == count_xml_escape_size(documentID));
     SkString instanceID = uuid_to_string(instance);
     SkASSERT(0 == count_xml_escape_size(instanceID));
-    return new PDFXMLObject(sk_string_printf(
+    return new PDFXMLObject(SkStringPrintf(
             templateString, modificationDate.c_str(), creationDate.c_str(),
             creator.c_str(), title.c_str(),
             subject.c_str(), author.c_str(), keywords1.c_str(),
diff --git a/tests/StringTest.cpp b/tests/StringTest.cpp
index 9e41c48c84..8ae7412187 100644
--- a/tests/StringTest.cpp
+++ b/tests/StringTest.cpp
@@ -148,6 +148,13 @@ DEF_TEST(String, reporter) {
     a.appendU64(0x0000000001000000ULL, 15);
     REPORTER_ASSERT(reporter, a.equals("000000016777216"));
 
+    a.printf("%i", 0);
+    REPORTER_ASSERT(reporter, a.equals("0"));
+    a.printf("%g", 3.14);
+    REPORTER_ASSERT(reporter, a.equals("3.14"));
+    a.printf("hello %s", "skia");
+    REPORTER_ASSERT(reporter, a.equals("hello skia"));
+
     static const struct {
         SkScalar    fValue;
         const char* fString;
@@ -185,6 +192,26 @@ DEF_TEST(String, reporter) {
     REPORTER_ASSERT(reporter, buffer[19] == 0);
     REPORTER_ASSERT(reporter, buffer[20] == 'a');
 
+    REPORTER_ASSERT(reporter, SkStringPrintf("%i", 0).equals("0"));
+
+    // 2000 is larger than the static buffer size inside SkString.cpp
+    a = SkStringPrintf("%2000s", " ");
+    REPORTER_ASSERT(reporter, a.size() == 2000);
+    for (size_t i = 0; i < a.size(); ++i) {
+        if (a[i] != ' ') {
+            ERRORF(reporter, "SkStringPrintf fail: a[%d] = '%c'", i, a[i]);
+            break;
+        }
+    }
+    a.reset();
+    a.printf("%2000s", " ");
+    REPORTER_ASSERT(reporter, a.size() == 2000);
+    for (size_t i = 0; i < a.size(); ++i) {
+        if (a[i] != ' ') {
+            ERRORF(reporter, "SkStringPrintf fail: a[%d] = '%c'", i, a[i]);
+            break;
+        }
+    }
 }
 
 DEF_TEST(String_SkStrSplit, r) {