Spanify CFX_SeekableStreamProxy
Remove some UNSAFE_BUFFERS introduced earlier in the CL chain.
-- add some missing "const" while at it.
-- improve variable naming.
Change-Id: Iba890ba329c320e7ce1fb74a1d83cd8478f14353
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118290
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fxcrt/cfx_seekablestreamproxy.cpp b/core/fxcrt/cfx_seekablestreamproxy.cpp
index d5d42fe..b223ad5 100644
--- a/core/fxcrt/cfx_seekablestreamproxy.cpp
+++ b/core/fxcrt/cfx_seekablestreamproxy.cpp
@@ -15,7 +15,6 @@
#include "build/build_config.h"
#include "core/fxcrt/check.h"
#include "core/fxcrt/check_op.h"
-#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/data_vector.h"
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_safe_types.h"
@@ -102,7 +101,7 @@
Seek(From::Begin, 0);
uint32_t bom = 0;
- ReadData(reinterpret_cast<uint8_t*>(&bom), 3);
+ ReadData(pdfium::byte_span_from_ref(bom).first(3));
bom &= BOM_UTF8_MASK;
if (bom == BOM_UTF8) {
@@ -127,15 +126,15 @@
CFX_SeekableStreamProxy::~CFX_SeekableStreamProxy() = default;
-FX_FILESIZE CFX_SeekableStreamProxy::GetSize() {
+FX_FILESIZE CFX_SeekableStreamProxy::GetSize() const {
return m_pStream->GetSize();
}
-FX_FILESIZE CFX_SeekableStreamProxy::GetPosition() {
+FX_FILESIZE CFX_SeekableStreamProxy::GetPosition() const {
return m_iPosition;
}
-bool CFX_SeekableStreamProxy::IsEOF() {
+bool CFX_SeekableStreamProxy::IsEOF() const {
return m_iPosition >= GetSize();
}
@@ -160,63 +159,51 @@
m_wCodePage = wCodePage;
}
-size_t CFX_SeekableStreamProxy::ReadData(uint8_t* pBuffer, size_t iBufferSize) {
- DCHECK(pBuffer);
- DCHECK(iBufferSize > 0);
-
- iBufferSize =
- std::min(iBufferSize, static_cast<size_t>(GetSize() - m_iPosition));
- if (iBufferSize <= 0)
- return 0;
-
- // SAFETY: required from caller.
- // TODO(tsepez): should be UNSAFE_BUFFER_USAGE.
- if (!m_pStream->ReadBlockAtOffset(UNSAFE_BUFFERS(
- pdfium::make_span(pBuffer, iBufferSize), m_iPosition))) {
+size_t CFX_SeekableStreamProxy::ReadData(pdfium::span<uint8_t> buffer) {
+ DCHECK(!buffer.empty());
+ const size_t remaining = static_cast<size_t>(GetSize() - m_iPosition);
+ size_t read_size = std::min(buffer.size(), remaining);
+ if (read_size == 0) {
return 0;
}
-
+ if (!m_pStream->ReadBlockAtOffset(buffer.first(read_size), m_iPosition)) {
+ return 0;
+ }
FX_SAFE_FILESIZE new_pos = m_iPosition;
- new_pos += iBufferSize;
+ new_pos += read_size;
m_iPosition = new_pos.ValueOrDefault(m_iPosition);
- return new_pos.IsValid() ? iBufferSize : 0;
+ return new_pos.IsValid() ? read_size : 0;
}
-// TODO(tsepez): should be UNSAFE_BUFFER_USAGE.
-size_t CFX_SeekableStreamProxy::ReadBlock(wchar_t* pStr, size_t size) {
- if (!pStr || size == 0)
+size_t CFX_SeekableStreamProxy::ReadBlock(pdfium::span<wchar_t> buffer) {
+ if (buffer.empty()) {
return 0;
-
+ }
if (m_wCodePage == FX_CodePage::kUTF16LE ||
m_wCodePage == FX_CodePage::kUTF16BE) {
- size_t iBytes = size * 2;
- size_t iLen = ReadData(reinterpret_cast<uint8_t*>(pStr), iBytes);
- size = iLen / 2;
+ size_t bytes_to_read = buffer.size() * sizeof(uint16_t);
+ size_t bytes_read =
+ ReadData(pdfium::as_writable_bytes(buffer).first(bytes_to_read));
+ size_t elements = bytes_read / sizeof(uint16_t);
if (m_wCodePage == FX_CodePage::kUTF16BE) {
- // SAFETY: required from caller.
- SwapByteOrder(UNSAFE_BUFFERS(
- pdfium::make_span(reinterpret_cast<uint16_t*>(pStr), size)));
+ SwapByteOrder(fxcrt::reinterpret_span<uint16_t>(buffer).first(elements));
}
- // SAFETY: required from caller.
- UTF16ToWChar(UNSAFE_BUFFERS(pdfium::make_span(pStr, size)));
- return size;
+ UTF16ToWChar(buffer.first(elements));
+ return elements;
}
-
FX_FILESIZE pos = GetPosition();
- size_t iBytes = std::min(size, static_cast<size_t>(GetSize() - pos));
- if (iBytes == 0)
+ size_t bytes_to_read =
+ std::min(buffer.size(), static_cast<size_t>(GetSize() - pos));
+ if (bytes_to_read == 0) {
return 0;
-
- DataVector<uint8_t> buf(iBytes);
- size_t iLen = ReadData(buf.data(), iBytes);
- if (m_wCodePage != FX_CodePage::kUTF8)
+ }
+ DataVector<uint8_t> byte_buf(bytes_to_read);
+ size_t bytes_read = ReadData(byte_buf);
+ if (m_wCodePage != FX_CodePage::kUTF8) {
return 0;
-
- size_t iSrc;
- // SAFETY: required from caller.
- std::tie(iSrc, size) =
- UTF8Decode(pdfium::make_span(buf).first(iLen),
- UNSAFE_BUFFERS(pdfium::make_span(pStr, size)));
- Seek(From::Current, iSrc - iLen);
- return size;
+ }
+ auto [src_bytes_consumed, dest_wchars_produced] =
+ UTF8Decode(pdfium::make_span(byte_buf).first(bytes_read), buffer);
+ Seek(From::Current, src_bytes_consumed - bytes_read);
+ return dest_wchars_produced;
}
diff --git a/core/fxcrt/cfx_seekablestreamproxy.h b/core/fxcrt/cfx_seekablestreamproxy.h
index 563134f..26f5b60 100644
--- a/core/fxcrt/cfx_seekablestreamproxy.h
+++ b/core/fxcrt/cfx_seekablestreamproxy.h
@@ -14,16 +14,17 @@
#include "core/fxcrt/fx_stream.h"
#include "core/fxcrt/fx_types.h"
#include "core/fxcrt/retain_ptr.h"
+#include "core/fxcrt/span.h"
class CFX_SeekableStreamProxy final : public Retainable {
public:
CONSTRUCT_VIA_MAKE_RETAIN;
- // Unlike IFX_SeekableStreamProxy, buffers and sizes are always in terms
- // of the number of wchar_t elementss, not bytes.
- FX_FILESIZE GetSize(); // Estimate under worst possible expansion.
- bool IsEOF();
- size_t ReadBlock(wchar_t* pStr, size_t size);
+ FX_FILESIZE GetSize() const; // Estimate under worst possible expansion.
+ bool IsEOF() const;
+
+ // Returns number of wchar_t elements placed into `buffer`.
+ size_t ReadBlock(pdfium::span<wchar_t> buffer);
FX_CodePage GetCodePage() const { return m_wCodePage; }
void SetCodePage(FX_CodePage wCodePage);
@@ -38,9 +39,9 @@
const RetainPtr<IFX_SeekableReadStream>& stream);
~CFX_SeekableStreamProxy() override;
- FX_FILESIZE GetPosition();
+ FX_FILESIZE GetPosition() const;
void Seek(From eSeek, FX_FILESIZE iOffset);
- size_t ReadData(uint8_t* pBuffer, size_t iBufferSize);
+ size_t ReadData(pdfium::span<uint8_t> buffer);
FX_CodePage m_wCodePage = FX_CodePage::kDefANSI;
size_t m_wBOMLength = 0;
diff --git a/core/fxcrt/cfx_seekablestreamproxy_unittest.cpp b/core/fxcrt/cfx_seekablestreamproxy_unittest.cpp
index 2c0df8e..e701e65 100644
--- a/core/fxcrt/cfx_seekablestreamproxy_unittest.cpp
+++ b/core/fxcrt/cfx_seekablestreamproxy_unittest.cpp
@@ -17,7 +17,7 @@
pdfium::span<const uint8_t>()));
wchar_t buffer[16];
- EXPECT_EQ(0u, proxy_stream->ReadBlock(buffer, std::size(buffer)));
+ EXPECT_EQ(0u, proxy_stream->ReadBlock(buffer));
}
TEST(SeekableStreamProxyTest, DefaultStreamBOMNotRecognized) {
@@ -26,7 +26,7 @@
pdfium::MakeRetain<CFX_ReadOnlySpanStream>(data.unsigned_span()));
wchar_t buffer[16];
- EXPECT_EQ(0u, proxy_stream->ReadBlock(buffer, std::size(buffer)));
+ EXPECT_EQ(0u, proxy_stream->ReadBlock(buffer));
}
TEST(SeekableStreamProxyTest, UTF8Stream) {
@@ -35,7 +35,7 @@
pdfium::MakeRetain<CFX_ReadOnlySpanStream>(data.unsigned_span()));
wchar_t buffer[16];
- EXPECT_EQ(3u, proxy_stream->ReadBlock(buffer, std::size(buffer)));
+ EXPECT_EQ(3u, proxy_stream->ReadBlock(buffer));
EXPECT_EQ(L'*', buffer[0]);
EXPECT_EQ(L'\u00A2', buffer[1]);
EXPECT_EQ(L'*', buffer[2]);
@@ -48,7 +48,7 @@
pdfium::MakeRetain<CFX_ReadOnlySpanStream>(data));
wchar_t buffer[16];
- EXPECT_EQ(2u, proxy_stream->ReadBlock(buffer, std::size(buffer)));
+ EXPECT_EQ(2u, proxy_stream->ReadBlock(buffer));
EXPECT_EQ(L'A', buffer[0]);
EXPECT_EQ(L'\u0142', buffer[1]);
}
@@ -59,7 +59,7 @@
pdfium::MakeRetain<CFX_ReadOnlySpanStream>(data));
wchar_t buffer[16];
- EXPECT_EQ(2u, proxy_stream->ReadBlock(buffer, std::size(buffer)));
+ EXPECT_EQ(2u, proxy_stream->ReadBlock(buffer));
EXPECT_EQ(L'A', buffer[0]);
EXPECT_EQ(L'\u0142', buffer[1]);
}
diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp
index 15c42f3..85357eb 100644
--- a/core/fxcrt/xml/cfx_xmlparser.cpp
+++ b/core/fxcrt/xml/cfx_xmlparser.cpp
@@ -120,7 +120,8 @@
if (stream_->IsEOF())
return true;
- size_t buffer_chars = stream_->ReadBlock(buffer.data(), xml_plane_size_);
+ size_t buffer_chars =
+ stream_->ReadBlock(pdfium::make_span(buffer).first(xml_plane_size_));
if (buffer_chars == 0)
return true;
