From 3361e6248b415bb3054b5af6462666f1608e0765 Mon Sep 17 00:00:00 2001 From: halcanary Date: Fri, 22 Apr 2016 08:19:04 -0700 Subject: [PATCH] Revert of SkStringPrintf and SkString::printf now are no longer limted by a static buffer (patchset #5 id:80001 of https://codereview.chromium.org/1403803002/ ) Reason for revert: breaking something Original issue's description: > 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 TBR=tomhudson@google.com,reed@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/1908423002 --- src/core/SkString.cpp | 61 +++++++---------------------------------------- src/pdf/SkPDFMetadata.cpp | 56 +++++++++++++++++++++++++++++++++++++++---- tests/StringTest.cpp | 26 -------------------- 3 files changed, 61 insertions(+), 82 deletions(-) diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp index 9331665..24b1b8f 100644 --- a/src/core/SkString.cpp +++ b/src/core/SkString.cpp @@ -33,56 +33,6 @@ 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); \ - output.resize((size_t)length); \ - va_start(args, format); \ - SkDEBUGCODE(int check = ) _vsnprintf_s(output.writable_str(), \ - length + 1, _TRUNCATE, \ - format, args); \ - va_end(args); \ - SkASSERT(check == 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; \ - } \ - output.resize((size_t)length); \ - va_start(args, format); \ - SkDEBUGCODE(int check = ) vsnprintf(output.writable_str(), \ - length + 1, format, args); \ - va_end(args); \ - SkASSERT(check == length); \ - SkASSERT(output[length] == '\0'); \ - } while (false) -#endif - /////////////////////////////////////////////////////////////////////////////// bool SkStrEndsWith(const char string[], const char suffixStr[]) { @@ -563,7 +513,11 @@ void SkString::insertScalar(size_t offset, SkScalar value) { } void SkString::printf(const char format[], ...) { - V_SKSTRING_PRINTF((*this), format); + char buffer[kBufferSize]; + int length; + ARGS_TO_BUFFER(format, buffer, kBufferSize, length); + + this->set(buffer, length); } void SkString::appendf(const char format[], ...) { @@ -639,7 +593,10 @@ void SkString::swap(SkString& other) { SkString SkStringPrintf(const char* format, ...) { SkString formattedOutput; - V_SKSTRING_PRINTF(formattedOutput, format); + char buffer[kBufferSize]; + SK_UNUSED int length; + ARGS_TO_BUFFER(format, buffer, kBufferSize, length); + formattedOutput.set(buffer); return formattedOutput; } diff --git a/src/pdf/SkPDFMetadata.cpp b/src/pdf/SkPDFMetadata.cpp index d3abd78..2765d4d 100644 --- a/src/pdf/SkPDFMetadata.cpp +++ b/src/pdf/SkPDFMetadata.cpp @@ -95,6 +95,54 @@ 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& info, const char* key) { for (const auto& keyValue : info) { @@ -255,14 +303,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 = SkStringPrintf("%s\n", - tmp.c_str()); + creationDate = sk_string_printf("%s\n", + tmp.c_str()); } if (fModified) { SkString tmp; fModified->toISO8601(&tmp); SkASSERT(0 == count_xml_escape_size(tmp)); - modificationDate = SkStringPrintf( + modificationDate = sk_string_printf( "%s\n", tmp.c_str()); } SkString title = escape_xml( @@ -291,7 +339,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(SkStringPrintf( + return new PDFXMLObject(sk_string_printf( 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 0e71c2b..9e41c48 100644 --- a/tests/StringTest.cpp +++ b/tests/StringTest.cpp @@ -148,13 +148,6 @@ 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; @@ -192,25 +185,6 @@ DEF_TEST(String, reporter) { REPORTER_ASSERT(reporter, buffer[19] == 0); REPORTER_ASSERT(reporter, buffer[20] == 'a'); - REPORTER_ASSERT(reporter, SkStringPrintf("%i", 0).equals("0")); - - 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) { -- 2.7.4