From 3f1c832dda209cf6682bb75316c07d71332fe6c3 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Thu, 16 Nov 2017 21:45:18 +0000 Subject: 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 Commit-Queue: dsinclair --- core/fpdfdoc/cpdf_pagelabel.cpp | 30 +++++----- core/fxcrt/widestring.cpp | 114 ++++++++++++++++++++----------------- core/fxcrt/widestring.h | 9 +-- core/fxcrt/widestring_unittest.cpp | 85 +++++---------------------- core/fxcrt/xml/cfx_xmldoc.cpp | 6 +- core/fxcrt/xml/cfx_xmlnode.cpp | 2 +- 6 files changed, 99 insertions(+), 147 deletions(-) (limited to 'core') 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 GuessSizeForVSWPrintf(const wchar_t* pFormat, return pdfium::Optional(nMaxLen); } +// Returns string unless we ran out of space. +pdfium::Optional 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(guess.value()); + } + + while (maxLen < 32 * 1024) { + va_copy(argListCopy, argList); + pdfium::Optional ret = + TryVSWPrintf(static_cast(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(guess.value()); - } - while (maxLen < 32 * 1024) { - va_copy(argListCopy, argList); - bool bSufficientBuffer = - TryVSWPrintf(static_cast(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; + 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 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"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"GetName().c_str()); + WideString ws = + WideString::Format(L"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"GetName().c_str()); + ws = WideString::Format(L"GetName().c_str()); pXMLStream->WriteString(ws.AsStringView()); for (auto it : pInstruction->GetAttributes()) { -- cgit v1.2.3