Return const pointer from CPDF_Stream::GetInMemoryRawData().
Callers to CPDF_Stream::GetInMemoryRawData() do not need the non-const
pointer to the data. GetInMemoryRawData() was only returning a non-const
pointer because fxcrt::MaybeOwned required it for the owned-data case.
In CPDF_StreamAcc, replace its MaybeOwned usage with absl::variant. Then
the variant can store either a read-only span, or some kind of owned
memory. Then CPDF_StreamAcc no longer need to have separate data and
size members.
As part of this CL, also restrict when GetInMemoryRawData() can be
called, and update CPDF_StreamAcc to match that behavior. Update the
comments for GetInMemoryRawData() to better explain how it is intended
to be used.
Change-Id: I6a05e8b5314434447e45c44596c81771765756fb
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97535
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 4bcd451..57ec20c 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -221,3 +221,8 @@
return true;
}
+
+const uint8_t* CPDF_Stream::GetInMemoryRawData() const {
+ DCHECK(IsMemoryBased());
+ return m_pDataBuf.get();
+}
diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h
index b6e4966..5da2398 100644
--- a/core/fpdfapi/parser/cpdf_stream.h
+++ b/core/fpdfapi/parser/cpdf_stream.h
@@ -35,9 +35,10 @@
const CPDF_Encryptor* encryptor) const override;
size_t GetRawSize() const { return m_dwSize; }
- // Will be null in case when stream is not memory based.
- // Use CPDF_StreamAcc to data access in all cases.
- uint8_t* GetInMemoryRawData() const { return m_pDataBuf.get(); }
+ // Can only be called when stream is memory-based.
+ // This is meant to be used by CPDF_StreamAcc only.
+ // Other callers should use CPDF_StreamAcc to access data in all cases.
+ const uint8_t* GetInMemoryRawData() const;
// Copies span or stream into internally-owned buffer.
void SetData(pdfium::span<const uint8_t> pData);
diff --git a/core/fpdfapi/parser/cpdf_stream_acc.cpp b/core/fpdfapi/parser/cpdf_stream_acc.cpp
index 485de02..27de211 100644
--- a/core/fpdfapi/parser/cpdf_stream_acc.cpp
+++ b/core/fpdfapi/parser/cpdf_stream_acc.cpp
@@ -13,7 +13,19 @@
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fpdfapi/parser/fpdf_parser_decode.h"
-#include "third_party/base/check.h"
+#include "third_party/base/check_op.h"
+
+CPDF_StreamAcc::OwnedData::OwnedData(
+ std::unique_ptr<uint8_t, FxFreeDeleter> buffer,
+ uint32_t size)
+ : buffer(std::move(buffer)), size(size) {}
+
+CPDF_StreamAcc::OwnedData::OwnedData(OwnedData&&) = default;
+
+CPDF_StreamAcc::OwnedData& CPDF_StreamAcc::OwnedData::operator=(OwnedData&&) =
+ default;
+
+CPDF_StreamAcc::OwnedData::~OwnedData() = default;
CPDF_StreamAcc::CPDF_StreamAcc(const CPDF_Stream* pStream)
: m_pStream(pStream) {}
@@ -60,14 +72,16 @@
}
const uint8_t* CPDF_StreamAcc::GetData() const {
- if (m_pData.IsOwned())
- return m_pData.Get();
- return m_pStream ? m_pStream->GetInMemoryRawData() : nullptr;
+ if (is_owned())
+ return absl::get<OwnedData>(m_Data).buffer.get();
+ return (m_pStream && m_pStream->IsMemoryBased())
+ ? m_pStream->GetInMemoryRawData()
+ : nullptr;
}
uint32_t CPDF_StreamAcc::GetSize() const {
- if (m_pData.IsOwned())
- return m_dwSize;
+ if (is_owned())
+ return absl::get<OwnedData>(m_Data).size;
return (m_pStream && m_pStream->IsMemoryBased()) ? m_pStream->GetRawSize()
: 0;
}
@@ -83,14 +97,16 @@
}
std::unique_ptr<uint8_t, FxFreeDeleter> CPDF_StreamAcc::DetachData() {
- if (m_pData.IsOwned()) {
- std::unique_ptr<uint8_t, FxFreeDeleter> p = m_pData.ReleaseAndClear();
- m_dwSize = 0;
- return p;
+ if (is_owned()) {
+ auto& data = absl::get<OwnedData>(m_Data);
+ data.size = 0;
+ return std::move(data.buffer);
}
- std::unique_ptr<uint8_t, FxFreeDeleter> p(FX_AllocUninit(uint8_t, m_dwSize));
- memcpy(p.get(), m_pData.Get(), m_dwSize);
- return p;
+ auto span = absl::get<pdfium::span<const uint8_t>>(m_Data);
+ std::unique_ptr<uint8_t, FxFreeDeleter> result(
+ FX_AllocUninit(uint8_t, span.size()));
+ memcpy(result.get(), span.data(), span.size());
+ return result;
}
void CPDF_StreamAcc::ProcessRawData() {
@@ -99,8 +115,7 @@
return;
if (m_pStream->IsMemoryBased()) {
- m_pData = m_pStream->GetInMemoryRawData();
- m_dwSize = dwSrcSize;
+ m_Data = pdfium::make_span(m_pStream->GetInMemoryRawData(), dwSrcSize);
return;
}
@@ -108,8 +123,7 @@
if (!pData)
return;
- m_pData = std::move(pData);
- m_dwSize = dwSrcSize;
+ m_Data.emplace<OwnedData>(std::move(pData), dwSrcSize);
}
void CPDF_StreamAcc::ProcessFilteredData(uint32_t estimated_size,
@@ -118,15 +132,18 @@
if (dwSrcSize == 0)
return;
- MaybeOwned<uint8_t, FxFreeDeleter> pSrcData;
+ absl::variant<pdfium::span<const uint8_t>, OwnedData> src_data;
+ pdfium::span<const uint8_t> src_span;
if (m_pStream->IsMemoryBased()) {
- pSrcData = m_pStream->GetInMemoryRawData();
+ src_span = pdfium::make_span(m_pStream->GetInMemoryRawData(), dwSrcSize);
+ src_data = src_span;
} else {
std::unique_ptr<uint8_t, FxFreeDeleter> pTempSrcData = ReadRawStream();
if (!pTempSrcData)
return;
- pSrcData = std::move(pTempSrcData);
+ src_span = pdfium::make_span(pTempSrcData.get(), dwSrcSize);
+ src_data.emplace<OwnedData>(std::move(pTempSrcData), dwSrcSize);
}
std::unique_ptr<uint8_t, FxFreeDeleter> pDecodedData;
@@ -135,21 +152,18 @@
absl::optional<std::vector<std::pair<ByteString, const CPDF_Object*>>>
decoder_array = GetDecoderArray(m_pStream->GetDict());
if (!decoder_array.has_value() || decoder_array.value().empty() ||
- !PDF_DataDecode({pSrcData.Get(), dwSrcSize}, estimated_size, bImageAcc,
+ !PDF_DataDecode(src_span, estimated_size, bImageAcc,
decoder_array.value(), &pDecodedData, &dwDecodedSize,
&m_ImageDecoder, &m_pImageParam)) {
- m_pData = std::move(pSrcData);
- m_dwSize = dwSrcSize;
+ m_Data = std::move(src_data);
return;
}
if (pDecodedData) {
- DCHECK(pDecodedData.get() != pSrcData.Get());
- m_pData = std::move(pDecodedData);
- m_dwSize = dwDecodedSize;
+ DCHECK_NE(pDecodedData.get(), src_span.data());
+ m_Data.emplace<OwnedData>(std::move(pDecodedData), dwDecodedSize);
} else {
- m_pData = std::move(pSrcData);
- m_dwSize = dwSrcSize;
+ m_Data = std::move(src_data);
}
}
diff --git a/core/fpdfapi/parser/cpdf_stream_acc.h b/core/fpdfapi/parser/cpdf_stream_acc.h
index c6b71f1..3314a0e 100644
--- a/core/fpdfapi/parser/cpdf_stream_acc.h
+++ b/core/fpdfapi/parser/cpdf_stream_acc.h
@@ -13,8 +13,8 @@
#include "core/fxcrt/bytestring.h"
#include "core/fxcrt/fx_memory_wrappers.h"
-#include "core/fxcrt/maybe_owned.h"
#include "core/fxcrt/retain_ptr.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
#include "third_party/base/span.h"
class CPDF_Dictionary;
@@ -43,6 +43,17 @@
std::unique_ptr<uint8_t, FxFreeDeleter> DetachData();
private:
+ // TODO(crbug.com/pdfium/1872): Replace with fxcrt::DataVector.
+ struct OwnedData {
+ OwnedData(std::unique_ptr<uint8_t, FxFreeDeleter> buffer, uint32_t size);
+ OwnedData(OwnedData&&);
+ OwnedData& operator=(OwnedData&&);
+ ~OwnedData();
+
+ std::unique_ptr<uint8_t, FxFreeDeleter> buffer;
+ uint32_t size;
+ };
+
explicit CPDF_StreamAcc(const CPDF_Stream* pStream);
~CPDF_StreamAcc() override;
@@ -54,8 +65,9 @@
// Reads the raw data from |m_pStream|, or return nullptr on failure.
std::unique_ptr<uint8_t, FxFreeDeleter> ReadRawStream() const;
- MaybeOwned<uint8_t, FxFreeDeleter> m_pData;
- uint32_t m_dwSize = 0;
+ bool is_owned() const { return m_Data.index() == 1; }
+
+ absl::variant<pdfium::span<const uint8_t>, OwnedData> m_Data;
ByteString m_ImageDecoder;
RetainPtr<const CPDF_Dictionary> m_pImageParam;
RetainPtr<const CPDF_Stream> const m_pStream;
