Replace pointer arithmetic with spans in CFX_BinaryBuf.
-- Add Append*() methods for numerics and test.
-- Add some checked_casts on lengths.
Change-Id: I0e45a90e5236fb0dc25279c7df513626d071160d
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/101470
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.cpp b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
index 761b16a..5404207 100644
--- a/core/fpdfapi/parser/cpdf_crypto_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
@@ -141,12 +141,12 @@
return false;
if (m_Cipher == Cipher::kNone) {
- dest_buf.AppendBlock(source.data(), source.size());
+ dest_buf.AppendSpan(source);
return true;
}
if (m_Cipher == Cipher::kRC4) {
size_t old_size = dest_buf.GetSize();
- dest_buf.AppendBlock(source.data(), source.size());
+ dest_buf.AppendSpan(source);
CRYPT_ArcFourCrypt(
static_cast<CRYPT_rc4_context*>(context),
dest_buf.GetMutableSpan().subspan(old_size, source.size()));
@@ -174,7 +174,7 @@
uint8_t block_buf[16];
CRYPT_AESDecrypt(&pContext->m_Context, block_buf, pContext->m_Block,
16);
- dest_buf.AppendBlock(block_buf, 16);
+ dest_buf.AppendSpan(block_buf);
pContext->m_BlockOffset = 0;
}
}
@@ -200,8 +200,9 @@
if (pContext->m_BlockOffset == 16) {
uint8_t block_buf[16];
CRYPT_AESDecrypt(&pContext->m_Context, block_buf, pContext->m_Block, 16);
- if (block_buf[15] <= 16) {
- dest_buf.AppendBlock(block_buf, 16 - block_buf[15]);
+ if (block_buf[15] < 16) {
+ dest_buf.AppendSpan(
+ pdfium::make_span(block_buf).first(16 - block_buf[15]));
}
}
FX_Free(pContext);
diff --git a/core/fxcodec/fax/faxmodule.cpp b/core/fxcodec/fax/faxmodule.cpp
index 84afeee..850a8cd 100644
--- a/core/fxcodec/fax/faxmodule.cpp
+++ b/core/fxcodec/fax/faxmodule.cpp
@@ -800,17 +800,18 @@
m_DestBitpos = 0;
uint8_t last_byte = 0;
for (int i = 0; i < m_Rows; ++i) {
+ pdfium::span<uint8_t> buf_span = pdfium::make_span(m_LineBuf);
+ fxcrt::spanset(buf_span, 0);
+ buf_span[0] = last_byte;
pdfium::span<const uint8_t> scan_line = m_Src->GetScanline(i);
- std::fill(std::begin(m_LineBuf), std::end(m_LineBuf), 0);
- m_LineBuf[0] = last_byte;
FaxEncode2DLine(scan_line);
- m_DestBuf.AppendBlock(m_LineBuf.data(), m_DestBitpos / 8);
+ m_DestBuf.AppendSpan(buf_span.first(m_DestBitpos / 8));
last_byte = m_LineBuf[m_DestBitpos / 8];
m_DestBitpos %= 8;
m_RefLineSpan = scan_line;
}
if (m_DestBitpos)
- m_DestBuf.AppendByte(last_byte);
+ m_DestBuf.AppendUint8(last_byte);
return m_DestBuf.DetachBuffer();
}
diff --git a/core/fxcrt/binary_buffer.cpp b/core/fxcrt/binary_buffer.cpp
index ef57934..341d4c0 100644
--- a/core/fxcrt/binary_buffer.cpp
+++ b/core/fxcrt/binary_buffer.cpp
@@ -10,6 +10,7 @@
#include <utility>
#include "core/fxcrt/fx_safe_types.h"
+#include "core/fxcrt/span_util.h"
namespace fxcrt {
@@ -42,8 +43,9 @@
if (m_buffer.empty() || count > GetSize() || start_index > GetSize() - count)
return;
- memmove(m_buffer.data() + start_index, m_buffer.data() + start_index + count,
- GetSize() - start_index - count);
+ auto buffer_span = pdfium::make_span(m_buffer).first(GetSize());
+ fxcrt::spanmove(buffer_span.subspan(start_index),
+ buffer_span.subspan(start_index + count));
m_DataSize -= count;
}
@@ -89,29 +91,32 @@
}
void BinaryBuffer::AppendSpan(pdfium::span<const uint8_t> span) {
- return AppendBlock(span.data(), span.size());
-}
-
-void BinaryBuffer::AppendBlock(const void* pBuf, size_t size) {
- if (size == 0)
+ if (span.empty())
return;
- ExpandBuf(size);
- if (pBuf) {
- memcpy(m_buffer.data() + GetSize(), pBuf, size);
- } else {
- memset(m_buffer.data() + GetSize(), 0, size);
- }
- m_DataSize += size;
+ ExpandBuf(span.size());
+ fxcrt::spancpy(pdfium::make_span(m_buffer).subspan(GetSize()), span);
+ m_DataSize += span.size();
}
void BinaryBuffer::AppendString(const ByteString& str) {
- AppendBlock(str.c_str(), str.GetLength());
+ AppendSpan(str.raw_span());
}
-void BinaryBuffer::AppendByte(uint8_t byte) {
- ExpandBuf(1);
- m_buffer[m_DataSize++] = byte;
+void BinaryBuffer::AppendUint8(uint8_t value) {
+ AppendSpan({&value, 1});
+}
+
+void BinaryBuffer::AppendUint16(uint16_t value) {
+ AppendSpan({reinterpret_cast<uint8_t*>(&value), sizeof(value)});
+}
+
+void BinaryBuffer::AppendUint32(uint32_t value) {
+ AppendSpan({reinterpret_cast<uint8_t*>(&value), sizeof(value)});
+}
+
+void BinaryBuffer::AppendDouble(double value) {
+ AppendSpan({reinterpret_cast<uint8_t*>(&value), sizeof(value)});
}
} // namespace fxcrt
diff --git a/core/fxcrt/binary_buffer.h b/core/fxcrt/binary_buffer.h
index 04463a3..9150266 100644
--- a/core/fxcrt/binary_buffer.h
+++ b/core/fxcrt/binary_buffer.h
@@ -38,9 +38,11 @@
void SetAllocStep(size_t step) { m_AllocStep = step; }
void EstimateSize(size_t size);
void AppendSpan(pdfium::span<const uint8_t> span);
- void AppendBlock(const void* pBuf, size_t size);
void AppendString(const ByteString& str);
- void AppendByte(uint8_t byte);
+ void AppendUint8(uint8_t value);
+ void AppendUint16(uint16_t value);
+ void AppendUint32(uint32_t value);
+ void AppendDouble(double value);
// Releases ownership of `m_pBuffer` and returns it.
DataVector<uint8_t> DetachBuffer();
diff --git a/core/fxcrt/binary_buffer_unittest.cpp b/core/fxcrt/binary_buffer_unittest.cpp
index 2cdbb2b..30a7e24 100644
--- a/core/fxcrt/binary_buffer_unittest.cpp
+++ b/core/fxcrt/binary_buffer_unittest.cpp
@@ -22,7 +22,7 @@
TEST(BinaryBuffer, MoveConstruct) {
BinaryBuffer buffer;
- buffer.AppendByte(65u);
+ buffer.AppendUint8(65u);
BinaryBuffer buffer2(std::move(buffer));
@@ -40,7 +40,7 @@
TEST(BinaryBuffer, MoveAssign) {
BinaryBuffer buffer;
BinaryBuffer buffer2;
- buffer.AppendByte(65u);
+ buffer.AppendUint8(65u);
buffer2 = std::move(buffer);
EXPECT_TRUE(buffer.IsEmpty());
@@ -56,7 +56,7 @@
TEST(BinaryBuffer, Clear) {
BinaryBuffer buffer;
- buffer.AppendByte(65u);
+ buffer.AppendUint8(65u);
buffer.Clear();
EXPECT_TRUE(buffer.IsEmpty());
EXPECT_EQ(0u, buffer.GetSize());
@@ -85,8 +85,8 @@
BinaryBuffer buffer;
std::vector<uint8_t> aaa(3, 65u);
std::vector<uint8_t> bbb(3, 66u);
- buffer.AppendBlock(aaa.data(), aaa.size());
- buffer.AppendBlock(bbb.data(), bbb.size());
+ buffer.AppendSpan(aaa);
+ buffer.AppendSpan(bbb);
EXPECT_EQ(6u, buffer.GetSize());
EXPECT_EQ(6u, buffer.GetLength());
EXPECT_EQ(65u, buffer.GetSpan()[0]);
@@ -111,12 +111,42 @@
TEST(BinaryBuffer, AppendBytes) {
BinaryBuffer buffer;
- buffer.AppendByte(65u);
- buffer.AppendByte(66u);
+ buffer.AppendUint8(65u);
+ buffer.AppendUint8(66u);
EXPECT_EQ(2u, buffer.GetSize());
EXPECT_EQ(2u, buffer.GetLength());
EXPECT_EQ(65u, buffer.GetSpan()[0]);
EXPECT_EQ(66u, buffer.GetSpan()[1]);
}
+// Assumes little endian.
+TEST(BinaryBuffer, AppendUint16) {
+ BinaryBuffer buffer;
+ buffer.AppendUint16(0x4321);
+ EXPECT_EQ(2u, buffer.GetSize());
+ EXPECT_EQ(2u, buffer.GetLength());
+ EXPECT_EQ(0x21u, buffer.GetSpan()[0]);
+ EXPECT_EQ(0x43u, buffer.GetSpan()[1]);
+}
+
+// Assumes little endian.
+TEST(BinaryBuffer, AppendUint32) {
+ BinaryBuffer buffer;
+ buffer.AppendUint32(0x87654321);
+ EXPECT_EQ(4u, buffer.GetSize());
+ EXPECT_EQ(4u, buffer.GetLength());
+ EXPECT_EQ(0x21u, buffer.GetSpan()[0]);
+ EXPECT_EQ(0x43u, buffer.GetSpan()[1]);
+ EXPECT_EQ(0x65u, buffer.GetSpan()[2]);
+ EXPECT_EQ(0x87u, buffer.GetSpan()[3]);
+}
+
+TEST(BinaryBuffer, AppendDouble) {
+ BinaryBuffer buffer;
+ buffer.AppendDouble(1234.5678);
+ EXPECT_EQ(8u, buffer.GetSize());
+ EXPECT_EQ(8u, buffer.GetLength());
+ // arch-dependent bit pattern.
+}
+
} // namespace fxcrt
diff --git a/core/fxcrt/widetext_buffer.cpp b/core/fxcrt/widetext_buffer.cpp
index a233875..145c056 100644
--- a/core/fxcrt/widetext_buffer.cpp
+++ b/core/fxcrt/widetext_buffer.cpp
@@ -42,6 +42,10 @@
DeleteBuf(start_index * sizeof(wchar_t), count * sizeof(wchar_t));
}
+void WideTextBuffer::AppendWideString(WideStringView str) {
+ AppendSpan(pdfium::as_bytes(str.span()));
+}
+
WideTextBuffer& WideTextBuffer::operator<<(ByteStringView ascii) {
pdfium::span<wchar_t> new_span = ExpandWideBuf(ascii.GetLength());
for (size_t i = 0; i < ascii.GetLength(); ++i)
@@ -50,22 +54,22 @@
}
WideTextBuffer& WideTextBuffer::operator<<(WideStringView str) {
- AppendBlock(str.unterminated_c_str(), str.GetLength() * sizeof(wchar_t));
+ AppendWideString(str);
return *this;
}
WideTextBuffer& WideTextBuffer::operator<<(const WideString& str) {
- AppendBlock(str.c_str(), str.GetLength() * sizeof(wchar_t));
+ AppendWideString(str.AsStringView());
return *this;
}
WideTextBuffer& WideTextBuffer::operator<<(const wchar_t* lpsz) {
- AppendBlock(lpsz, wcslen(lpsz) * sizeof(wchar_t));
+ AppendWideString(WideStringView(lpsz));
return *this;
}
WideTextBuffer& WideTextBuffer::operator<<(const WideTextBuffer& buf) {
- AppendBlock(buf.m_buffer.data(), buf.GetSize());
+ AppendWideString(buf.AsStringView());
return *this;
}
diff --git a/core/fxcrt/widetext_buffer.h b/core/fxcrt/widetext_buffer.h
index ff5d644..a02d128 100644
--- a/core/fxcrt/widetext_buffer.h
+++ b/core/fxcrt/widetext_buffer.h
@@ -35,6 +35,8 @@
WideTextBuffer& operator<<(const WideTextBuffer& buf);
private:
+ void AppendWideString(WideStringView str);
+
// Returned span is the newly-expanded space.
pdfium::span<wchar_t> ExpandWideBuf(size_t char_count);
};
diff --git a/fxjs/cfx_globaldata.cpp b/fxjs/cfx_globaldata.cpp
index ee58cca..59b43c1 100644
--- a/fxjs/cfx_globaldata.cpp
+++ b/fxjs/cfx_globaldata.cpp
@@ -41,12 +41,10 @@
void MakeNameTypeString(const ByteString& name,
CFX_Value::DataType eType,
BinaryBuffer* result) {
- uint32_t dwNameLen = (uint32_t)name.GetLength();
- result->AppendBlock(&dwNameLen, sizeof(uint32_t));
+ uint32_t dwNameLen = pdfium::base::checked_cast<uint32_t>(name.GetLength());
+ result->AppendUint32(dwNameLen);
result->AppendString(name);
-
- uint16_t wType = static_cast<uint16_t>(eType);
- result->AppendBlock(&wType, sizeof(uint16_t));
+ result->AppendUint16(static_cast<uint16_t>(eType));
}
bool MakeByteString(const ByteString& name,
@@ -55,20 +53,19 @@
switch (pData.nType) {
case CFX_Value::DataType::kNumber: {
MakeNameTypeString(name, pData.nType, result);
- double dData = pData.dData;
- result->AppendBlock(&dData, sizeof(double));
+ result->AppendDouble(pData.dData);
return true;
}
case CFX_Value::DataType::kBoolean: {
MakeNameTypeString(name, pData.nType, result);
- uint16_t wData = static_cast<uint16_t>(pData.bData);
- result->AppendBlock(&wData, sizeof(uint16_t));
+ result->AppendUint16(static_cast<uint16_t>(pData.bData));
return true;
}
case CFX_Value::DataType::kString: {
MakeNameTypeString(name, pData.nType, result);
- uint32_t dwDataLen = (uint32_t)pData.sData.GetLength();
- result->AppendBlock(&dwDataLen, sizeof(uint32_t));
+ uint32_t dwDataLen =
+ pdfium::base::checked_cast<uint32_t>(pData.sData.GetLength());
+ result->AppendUint32(dwDataLen);
result->AppendString(pData.sData);
return true;
}
@@ -379,14 +376,12 @@
}
BinaryBuffer sFile;
- uint16_t wType = kMagic;
- uint16_t wVersion = 2;
- sFile.AppendBlock(&wType, sizeof(uint16_t));
- sFile.AppendBlock(&wVersion, sizeof(uint16_t));
- sFile.AppendBlock(&nCount, sizeof(uint32_t));
+ sFile.AppendUint16(kMagic);
+ sFile.AppendUint16(kMaxVersion);
+ sFile.AppendUint32(nCount);
uint32_t dwSize = pdfium::base::checked_cast<uint32_t>(sData.GetSize());
- sFile.AppendBlock(&dwSize, sizeof(uint32_t));
+ sFile.AppendUint32(dwSize);
sFile.AppendSpan(sData.GetSpan());
CRYPT_ArcFourCryptBlock(sFile.GetMutableSpan(), kRC4KEY);
