From b7973bb5a0ad2f83a71362d601a928964559004a Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 17 Apr 2018 16:41:28 +0000 Subject: Reland "Return pdfium::span from WideString::GetBuffer()."" This reverts commit 0d32b8fda53e02c1036d39f7290d4f59f2b58ca4. Restore behaviour on trunk. TBR: dsinclair@chromium.org Change-Id: Ia867f09ae9d2885595c4d9b300a058431dfd84f2 Reviewed-on: https://pdfium-review.googlesource.com/30811 Reviewed-by: Tom Sepez Commit-Queue: Tom Sepez --- core/fpdfapi/parser/fpdf_parser_decode.cpp | 11 +++-- core/fxcrt/cfx_blockbuffer.cpp | 55 +++++++++++++------------ core/fxcrt/widestring.cpp | 65 +++++++++++++++++------------- core/fxcrt/widestring.h | 7 +++- core/fxcrt/widestring_unittest.cpp | 21 +++++----- 5 files changed, 87 insertions(+), 72 deletions(-) (limited to 'core') diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp index d7114b66c5..90dca2edcb 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp @@ -425,6 +425,7 @@ bool PDF_DataDecode(const uint8_t* src_buf, } WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { + int dest_pos = 0; WideString result; if (src_len >= 2 && ((src_data[0] == 0xfe && src_data[1] == 0xff) || (src_data[0] == 0xff && src_data[1] == 0xfe))) { @@ -432,17 +433,15 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { if (!max_chars) return result; + pdfium::span dest_buf = result.GetBuffer(max_chars); bool bBE = src_data[0] == 0xfe || (src_data[0] == 0xff && !src_data[2]); - wchar_t* dest_buf = result.GetBuffer(max_chars); const uint8_t* uni_str = src_data + 2; - int dest_pos = 0; for (uint32_t i = 0; i < max_chars * 2; i += 2) { uint16_t unicode = GetUnicodeFromBytes(uni_str + i, bBE); if (unicode != 0x1b) { dest_buf[dest_pos++] = unicode; continue; } - i += 2; while (i < max_chars * 2) { uint16_t unicode2 = GetUnicodeFromBytes(uni_str + i, bBE); @@ -451,13 +450,13 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { break; } } - result.ReleaseBuffer(dest_pos); } else { - wchar_t* dest_buf = result.GetBuffer(src_len); + pdfium::span dest_buf = result.GetBuffer(src_len); for (uint32_t i = 0; i < src_len; ++i) dest_buf[i] = PDFDocEncoding[src_data[i]]; - result.ReleaseBuffer(src_len); + dest_pos = src_len; } + result.ReleaseBuffer(dest_pos); return result; } diff --git a/core/fxcrt/cfx_blockbuffer.cpp b/core/fxcrt/cfx_blockbuffer.cpp index 13134f0e7b..6a7d98aa18 100644 --- a/core/fxcrt/cfx_blockbuffer.cpp +++ b/core/fxcrt/cfx_blockbuffer.cpp @@ -77,36 +77,39 @@ WideString CFX_BlockBuffer::GetTextData(size_t start, size_t length) const { size_t maybeDataLength = m_BufferSize - 1 - m_StartPosition; if (start > maybeDataLength) return WideString(); - length = std::min(length, maybeDataLength); - WideString wsTextData; - wchar_t* pBuf = wsTextData.GetBuffer(length); - if (!pBuf) + length = std::min(length, maybeDataLength); + if (!length) return WideString(); - size_t startBlock = 0; - size_t startInner = 0; - std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start); - - size_t endBlock = 0; - size_t endInner = 0; - std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length); - - size_t pointer = 0; - for (size_t i = startBlock; i <= endBlock; ++i) { - size_t bufferPointer = 0; - size_t copyLength = kAllocStep; - if (i == startBlock) { - copyLength -= startInner; - bufferPointer = startInner; + WideString wsTextData; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span pBuf = wsTextData.GetBuffer(length); + size_t startBlock = 0; + size_t startInner = 0; + std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start); + + size_t endBlock = 0; + size_t endInner = 0; + std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length); + + size_t pointer = 0; + for (size_t i = startBlock; i <= endBlock; ++i) { + size_t bufferPointer = 0; + size_t copyLength = kAllocStep; + if (i == startBlock) { + copyLength -= startInner; + bufferPointer = startInner; + } + if (i == endBlock) + copyLength -= ((kAllocStep - 1) - endInner); + + wchar_t* pBlockBuf = m_BlockArray[i].get(); + memcpy(&pBuf[pointer], pBlockBuf + bufferPointer, + copyLength * sizeof(wchar_t)); + pointer += copyLength; } - if (i == endBlock) - copyLength -= ((kAllocStep - 1) - endInner); - - wchar_t* pBlockBuf = m_BlockArray[i].get(); - memcpy(pBuf + pointer, pBlockBuf + bufferPointer, - copyLength * sizeof(wchar_t)); - pointer += copyLength; } wsTextData.ReleaseBuffer(length); return wsTextData; diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp index 7b5bf66fd3..a3525593ee 100644 --- a/core/fxcrt/widestring.cpp +++ b/core/fxcrt/widestring.cpp @@ -252,22 +252,27 @@ Optional GuessSizeForVSWPrintf(const wchar_t* pFormat, 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) + if (!size) return {}; + WideString str; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span buffer = str.GetBuffer(size); + + // In the following two calls, there's always space in the WideString + // for a terminating NUL that's not included in the span. + // 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.data(), 0, (size + 1) * sizeof(wchar_t)); + int ret = vswprintf(buffer.data(), size + 1, pFormat, argList); + + bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0; + if (!bSufficientBuffer) + return {}; + } str.ReleaseBuffer(str.GetStringLength()); return {str}; } @@ -299,9 +304,12 @@ WideString GetWideString(uint16_t codepage, const ByteStringView& bstr) { return WideString(); WideString wstr; - wchar_t* dest_buf = wstr.GetBuffer(dest_len); - FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len, - dest_buf, dest_len); + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span dest_buf = wstr.GetBuffer(dest_len); + FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len, + dest_buf.data(), dest_len); + } wstr.ReleaseBuffer(dest_len); return wstr; } @@ -586,29 +594,29 @@ void WideString::Reserve(size_t len) { GetBuffer(len); } -wchar_t* WideString::GetBuffer(size_t nMinBufLength) { +pdfium::span WideString::GetBuffer(size_t nMinBufLength) { if (!m_pData) { if (nMinBufLength == 0) - return nullptr; + return pdfium::span(); m_pData.Reset(StringData::Create(nMinBufLength)); m_pData->m_nDataLength = 0; m_pData->m_String[0] = 0; - return m_pData->m_String; + return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); } if (m_pData->CanOperateInPlace(nMinBufLength)) - return m_pData->m_String; + return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength); if (nMinBufLength == 0) - return nullptr; + return pdfium::span(); RetainPtr pNewData(StringData::Create(nMinBufLength)); pNewData->CopyContents(*m_pData); pNewData->m_nDataLength = m_pData->m_nDataLength; m_pData.Swap(pNewData); - return m_pData->m_String; + return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); } size_t WideString::Delete(size_t index, size_t count) { @@ -885,14 +893,15 @@ WideString WideString::FromUTF8(const ByteStringView& str) { // static WideString WideString::FromUTF16LE(const unsigned short* wstr, size_t wlen) { - if (!wstr || wlen == 0) { + if (!wstr || wlen == 0) return WideString(); - } WideString result; - wchar_t* buf = result.GetBuffer(wlen); - for (size_t i = 0; i < wlen; i++) { - buf[i] = wstr[i]; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span buf = result.GetBuffer(wlen); + for (size_t i = 0; i < wlen; i++) + buf[i] = wstr[i]; } result.ReleaseBuffer(wlen); return result; diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h index cefdfabcd0..b668b0292c 100644 --- a/core/fxcrt/widestring.h +++ b/core/fxcrt/widestring.h @@ -17,7 +17,7 @@ #include "core/fxcrt/string_data_template.h" #include "core/fxcrt/string_view_template.h" #include "third_party/base/optional.h" - +#include "third_party/base/span.h" namespace fxcrt { @@ -171,7 +171,10 @@ class WideString { void TrimRight(const WideStringView& targets); void Reserve(size_t len); - wchar_t* GetBuffer(size_t len); + + // Note: any modification of the string (including ReleaseBuffer()) may + // invalidate the span, which must not outlive its buffer. + pdfium::span GetBuffer(size_t len); void ReleaseBuffer(size_t len); int GetInteger() const; diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp index 48d5c7f8f5..ec0a55fc1e 100644 --- a/core/fxcrt/widestring_unittest.cpp +++ b/core/fxcrt/widestring_unittest.cpp @@ -817,20 +817,21 @@ TEST(WideString, Reserve) { } TEST(WideString, GetBuffer) { + WideString str1; { - WideString str; - wchar_t* buffer = str.GetBuffer(12); - wcscpy(buffer, L"clams"); - str.ReleaseBuffer(str.GetStringLength()); - EXPECT_EQ(L"clams", str); + pdfium::span buffer = str1.GetBuffer(12); + wcscpy(buffer.data(), L"clams"); } + str1.ReleaseBuffer(str1.GetStringLength()); + EXPECT_EQ(L"clams", str1); + + WideString str2(L"cl"); { - WideString str(L"cl"); - wchar_t* buffer = str.GetBuffer(12); - wcscpy(buffer + 2, L"ams"); - str.ReleaseBuffer(str.GetStringLength()); - EXPECT_EQ(L"clams", str); + pdfium::span buffer = str2.GetBuffer(12); + wcscpy(buffer.data() + 2, L"ams"); } + str2.ReleaseBuffer(str2.GetStringLength()); + EXPECT_EQ(L"clams", str2); } TEST(WideString, ReleaseBuffer) { -- cgit v1.2.3