Make WideString::{Format|FormatV} static

This CL moves the Format and FormatV methods from WideString to be
static.

Bug: pdfium:934
Change-Id: I9941d6a2a5bbf0a82087cd0ea5d0f8fc42eecd3e
Reviewed-on: https://pdfium-review.googlesource.com/18630
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

6 files changed, 99 insertions, 147 deletions

diff --git a/core/fpdfdoc/cpdf_pagelabel.cpp b/core/fpdfdoc/cpdf_pagelabel.cpp
index 5efb18ef3d..d0693086ab 100644
--- a/core/fpdfdoc/cpdf_pagelabel.cpp
+++ b/core/fpdfdoc/cpdf_pagelabel.cpp
@@ -50,23 +50,25 @@ WideString MakeLetters(int num) {
 }
 
 WideString GetLabelNumPortion(int num, const ByteString& bsStyle) {
-  WideString wsNumPortion;
   if (bsStyle.IsEmpty())
-    return wsNumPortion;
-  if (bsStyle == "D") {
-    wsNumPortion.Format(L"%d", num);
-  } else if (bsStyle == "R") {
-    wsNumPortion = MakeRoman(num);
+    return L"";
+  if (bsStyle == "D")
+    return WideString::Format(L"%d", num);
+  if (bsStyle == "R") {
+    WideString wsNumPortion = MakeRoman(num);
     wsNumPortion.MakeUpper();
-  } else if (bsStyle == "r") {
-    wsNumPortion = MakeRoman(num);
-  } else if (bsStyle == "A") {
-    wsNumPortion = MakeLetters(num);
+    return wsNumPortion;
+  }
+  if (bsStyle == "r")
+    return MakeRoman(num);
+  if (bsStyle == "A") {
+    WideString wsNumPortion = MakeLetters(num);
     wsNumPortion.MakeUpper();
-  } else if (bsStyle == "a") {
-    wsNumPortion = MakeLetters(num);
+    return wsNumPortion;
   }
-  return wsNumPortion;
+  if (bsStyle == "a")
+    return MakeLetters(num);
+  return L"";
 }
 
 }  // namespace
@@ -114,7 +116,7 @@ bool CPDF_PageLabel::GetLabel(int nPage, WideString* wsLabel) const {
       return true;
     }
   }
-  wsLabel->Format(L"%d", nPage + 1);
+  *wsLabel = WideString::Format(L"%d", nPage + 1);
   return true;
 }
 
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index bd504e0cac..d33ed1ad6c 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -246,6 +246,30 @@ pdfium::Optional<size_t> GuessSizeForVSWPrintf(const wchar_t* pFormat,
   return pdfium::Optional<size_t>(nMaxLen);
 }
 
+// Returns string unless we ran out of space.
+pdfium::Optional<WideString> TryVSWPrintf(size_t size,
+                                          const wchar_t* pFormat,
+                                          va_list argList) {
+  WideString str;
+  wchar_t* buffer = str.GetBuffer(size);
+
+  // In the following two calls, there's always space in the buffer for
+  // a terminating NUL that's not included in nMaxLen.
+  // For vswprintf(), MSAN won't untaint the buffer on a truncated write's
+  // -1 return code even though the buffer is written. Probably just as well
+  // not to trust the vendor's implementation to write anything anyways.
+  // See https://crbug.com/705912.
+  memset(buffer, 0, (size + 1) * sizeof(wchar_t));
+  int ret = vswprintf(buffer, size + 1, pFormat, argList);
+
+  bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0;
+  if (!bSufficientBuffer)
+    return {};
+
+  str.ReleaseBuffer(str.GetStringLength());
+  return {str};
+}
+
 #ifndef NDEBUG
 bool IsValidCodePage(uint16_t codepage) {
   switch (codepage) {
@@ -285,6 +309,45 @@ namespace fxcrt {
 static_assert(sizeof(WideString) <= sizeof(wchar_t*),
               "Strings must not require more space than pointers");
 
+// static
+WideString WideString::FormatV(const wchar_t* format, va_list argList) {
+  va_list argListCopy;
+  va_copy(argListCopy, argList);
+  int maxLen = vswprintf(nullptr, 0, format, argListCopy);
+  va_end(argListCopy);
+
+  if (maxLen <= 0) {
+    va_copy(argListCopy, argList);
+    auto guess = GuessSizeForVSWPrintf(format, argListCopy);
+    va_end(argListCopy);
+
+    if (!guess.has_value())
+      return L"";
+    maxLen = pdfium::base::checked_cast<int>(guess.value());
+  }
+
+  while (maxLen < 32 * 1024) {
+    va_copy(argListCopy, argList);
+    pdfium::Optional<WideString> ret =
+        TryVSWPrintf(static_cast<size_t>(maxLen), format, argListCopy);
+    va_end(argListCopy);
+
+    if (ret)
+      return *ret;
+    maxLen *= 2;
+  }
+  return L"";
+}
+
+// static
+WideString WideString::Format(const wchar_t* pFormat, ...) {
+  va_list argList;
+  va_start(argList, pFormat);
+  WideString ret = FormatV(pFormat, argList);
+  va_end(argList);
+  return ret;
+}
+
 WideString::WideString() {}
 
 WideString::WideString(const WideString& other) : m_pData(other.m_pData) {}
@@ -666,57 +729,6 @@ void WideString::AllocCopy(WideString& dest,
   dest.m_pData.Swap(pNewData);
 }
 
-bool WideString::TryVSWPrintf(size_t size,
-                              const wchar_t* pFormat,
-                              va_list argList) {
-  GetBuffer(size);
-  if (!m_pData)
-    return true;
-
-  // In the following two calls, there's always space in the buffer for
-  // a terminating NUL that's not included in nMaxLen.
-  // For vswprintf(), MSAN won't untaint the buffer on a truncated write's
-  // -1 return code even though the buffer is written. Probably just as well
-  // not to trust the vendor's implementation to write anything anyways.
-  // See https://crbug.com/705912.
-  memset(m_pData->m_String, 0, (size + 1) * sizeof(wchar_t));
-  int ret = vswprintf(m_pData->m_String, size + 1, pFormat, argList);
-  bool bSufficientBuffer = ret >= 0 || m_pData->m_String[size - 1] == 0;
-  ReleaseBuffer(GetStringLength());
-  return bSufficientBuffer;
-}
-
-void WideString::FormatV(const wchar_t* format, va_list argList) {
-  va_list argListCopy;
-  va_copy(argListCopy, argList);
-  int maxLen = vswprintf(nullptr, 0, format, argListCopy);
-  va_end(argListCopy);
-  if (maxLen <= 0) {
-    va_copy(argListCopy, argList);
-    auto guess = GuessSizeForVSWPrintf(format, argListCopy);
-    va_end(argListCopy);
-    if (!guess.has_value())
-      return;
-    maxLen = pdfium::base::checked_cast<int>(guess.value());
-  }
-  while (maxLen < 32 * 1024) {
-    va_copy(argListCopy, argList);
-    bool bSufficientBuffer =
-        TryVSWPrintf(static_cast<size_t>(maxLen), format, argListCopy);
-    va_end(argListCopy);
-    if (bSufficientBuffer)
-      break;
-    maxLen *= 2;
-  }
-}
-
-void WideString::Format(const wchar_t* pFormat, ...) {
-  va_list argList;
-  va_start(argList, pFormat);
-  FormatV(pFormat, argList);
-  va_end(argList);
-}
-
 size_t WideString::Insert(size_t location, wchar_t ch) {
   const size_t cur_length = m_pData ? m_pData->m_nDataLength : 0;
   if (!IsValidLength(location))
diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h
index 01c4eedaf1..9d856c72c9 100644
--- a/core/fxcrt/widestring.h
+++ b/core/fxcrt/widestring.h
@@ -33,6 +33,9 @@ class WideString {
   using const_iterator = const CharType*;
   using const_reverse_iterator = std::reverse_iterator<const_iterator>;
 
+  static WideString Format(const wchar_t* lpszFormat, ...);
+  static WideString FormatV(const wchar_t* lpszFormat, va_list argList);
+
   WideString();
   WideString(const WideString& other);
   WideString(WideString&& other) noexcept;
@@ -141,9 +144,6 @@ class WideString {
   size_t InsertAtBack(wchar_t ch) { return Insert(GetLength(), ch); }
   size_t Delete(size_t index, size_t count = 1);
 
-  void Format(const wchar_t* lpszFormat, ...);
-  void FormatV(const wchar_t* lpszFormat, va_list argList);
-
   void MakeLower();
   void MakeUpper();
 
@@ -189,9 +189,6 @@ class WideString {
   void AssignCopy(const wchar_t* pSrcData, size_t nSrcLen);
   void Concat(const wchar_t* lpszSrcData, size_t nSrcLen);
 
-  // Returns true unless we ran out of space.
-  bool TryVSWPrintf(size_t size, const wchar_t* format, va_list argList);
-
   RetainPtr<StringData> m_pData;
 
   friend WideString_ConcatInPlace_Test;
diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index 42819b3349..aaa6d9cae7 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -1278,84 +1278,25 @@ TEST(WideStringView, TrimmedRight) {
 }
 
 TEST(WideString, FormatWidth) {
-  {
-    WideString str;
-    str.Format(L"%5d", 1);
-    EXPECT_EQ(L"    1", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%d", 1);
-    EXPECT_EQ(L"1", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%*d", 5, 1);
-    EXPECT_EQ(L"    1", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%-1d", 1);
-    EXPECT_EQ(L"1", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%0d", 1);
-    EXPECT_EQ(L"1", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%1048576d", 1);
-    EXPECT_EQ(L"", str);
-  }
+  EXPECT_EQ(L"    1", WideString::Format(L"%5d", 1));
+  EXPECT_EQ(L"1", WideString::Format(L"%d", 1));
+  EXPECT_EQ(L"    1", WideString::Format(L"%*d", 5, 1));
+  EXPECT_EQ(L"1", WideString::Format(L"%-1d", 1));
+  EXPECT_EQ(L"1", WideString::Format(L"%0d", 1));
+  EXPECT_EQ(L"", WideString::Format(L"%1048576d", 1));
 }
 
 TEST(WideString, FormatPrecision) {
-  {
-    WideString str;
-    str.Format(L"%.2f", 1.12345);
-    EXPECT_EQ(L"1.12", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%.*f", 3, 1.12345);
-    EXPECT_EQ(L"1.123", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%f", 1.12345);
-    EXPECT_EQ(L"1.123450", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%-1f", 1.12345);
-    EXPECT_EQ(L"1.123450", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%0f", 1.12345);
-    EXPECT_EQ(L"1.123450", str);
-  }
-
-  {
-    WideString str;
-    str.Format(L"%.1048576f", 1.2);
-    EXPECT_EQ(L"", str);
-  }
+  EXPECT_EQ(L"1.12", WideString::Format(L"%.2f", 1.12345));
+  EXPECT_EQ(L"1.123", WideString::Format(L"%.*f", 3, 1.12345));
+  EXPECT_EQ(L"1.123450", WideString::Format(L"%f", 1.12345));
+  EXPECT_EQ(L"1.123450", WideString::Format(L"%-1f", 1.12345));
+  EXPECT_EQ(L"1.123450", WideString::Format(L"%0f", 1.12345));
+  EXPECT_EQ(L"", WideString::Format(L"%.1048576f", 1.2));
 }
 
 TEST(WideString, FormatOutOfRangeChar) {
-  WideString str;
-  str.Format(L"unsupported char '%c'", 0x00FF00FF);
+  WideString::Format(L"unsupported char '%c'", 0x00FF00FF);
 }
 
 TEST(WideString, Empty) {
diff --git a/core/fxcrt/xml/cfx_xmldoc.cpp b/core/fxcrt/xml/cfx_xmldoc.cpp
index 30e4a47e72..e29c09a08e 100644
--- a/core/fxcrt/xml/cfx_xmldoc.cpp
+++ b/core/fxcrt/xml/cfx_xmldoc.cpp
@@ -53,10 +53,9 @@ void CFX_XMLDoc::SaveXMLNode(
   CFX_XMLNode* pNode = (CFX_XMLNode*)pINode;
   switch (pNode->GetType()) {
     case FX_XMLNODE_Instruction: {
-      WideString ws;
       CFX_XMLInstruction* pInstruction = (CFX_XMLInstruction*)pNode;
       if (pInstruction->GetName().CompareNoCase(L"xml") == 0) {
-        ws = L"<?xml version=\"1.0\" encoding=\"";
+        WideString ws = L"<?xml version=\"1.0\" encoding=\"";
         uint16_t wCodePage = pXMLStream->GetCodePage();
         if (wCodePage == FX_CODEPAGE_UTF16LE) {
           ws += L"UTF-16";
@@ -68,7 +67,8 @@ void CFX_XMLDoc::SaveXMLNode(
         ws += L"\"?>";
         pXMLStream->WriteString(ws.AsStringView());
       } else {
-        ws.Format(L"<?%s", pInstruction->GetName().c_str());
+        WideString ws =
+            WideString::Format(L"<?%s", pInstruction->GetName().c_str());
         pXMLStream->WriteString(ws.AsStringView());
 
         for (auto it : pInstruction->GetAttributes()) {
diff --git a/core/fxcrt/xml/cfx_xmlnode.cpp b/core/fxcrt/xml/cfx_xmlnode.cpp
index 4550d5b4ae..f2b9006ebf 100644
--- a/core/fxcrt/xml/cfx_xmlnode.cpp
+++ b/core/fxcrt/xml/cfx_xmlnode.cpp
@@ -349,7 +349,7 @@ void CFX_XMLNode::SaveXMLNode(
         ws += L"\"?>";
         pXMLStream->WriteString(ws.AsStringView());
       } else {
-        ws.Format(L"<?%s", pInstruction->GetName().c_str());
+        ws = WideString::Format(L"<?%s", pInstruction->GetName().c_str());
         pXMLStream->WriteString(ws.AsStringView());
 
         for (auto it : pInstruction->GetAttributes()) {