Use retained locals to avoid implicit .Get() calls
Avoid investigating future spurious warnings of the type identified in
https://pdfium-review.googlesource.com/c/pdfium/+/100612
-- Add missing form of ToName() now required.
-- convert some pointers to const ref for iterators.
Change-Id: I367c2c6c66d72f75afc64f81b1c196b97c45f9fd
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/100671
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index 1f33fce..3c9675e 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -412,7 +412,7 @@
CPDF_DictionaryLocker locker(m_pParser->GetCombinedTrailer());
for (const auto& it : locker) {
const ByteString& key = it.first;
- CPDF_Object* pValue = it.second.Get();
+ const RetainPtr<CPDF_Object>& pValue = it.second;
if (key == "Encrypt" || key == "Size" || key == "Filter" ||
key == "Index" || key == "Length" || key == "Prev" || key == "W" ||
key == "XRefStm" || key == "ID" || key == "DecodeParms" ||
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index 0b42c75..d15797e 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -160,7 +160,7 @@
continue;
}
- CPDF_Stream* old_stream =
+ RetainPtr<CPDF_Stream> old_stream =
page_content_manager.GetStreamByIndex(stream_index);
DCHECK(old_stream);
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 38317f5..ccb6f53 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -480,7 +480,7 @@
CPDF_DictionaryLocker locker(std::move(pDict));
for (const auto& it : locker) {
- CPDF_Name* pValue = ToName(it.second.Get());
+ RetainPtr<const CPDF_Name> pValue = ToName(it.second);
if (pValue) {
RetainPtr<CPDF_ColorSpace> pRet =
GetStockCSForName(pValue->GetString());
diff --git a/core/fpdfapi/page/cpdf_tilingpattern.cpp b/core/fpdfapi/page/cpdf_tilingpattern.cpp
index 8a06fa5..d7b2fd3 100644
--- a/core/fpdfapi/page/cpdf_tilingpattern.cpp
+++ b/core/fpdfapi/page/cpdf_tilingpattern.cpp
@@ -39,13 +39,13 @@
m_XStep = fabsf(pDict->GetFloatFor("XStep"));
m_YStep = fabsf(pDict->GetFloatFor("YStep"));
- CPDF_Stream* pStream = pattern_obj()->AsMutableStream();
+ RetainPtr<CPDF_Stream> pStream = ToStream(pattern_obj());
if (!pStream)
return nullptr;
const CFX_Matrix& matrix = parent_matrix();
- auto form = std::make_unique<CPDF_Form>(document(), nullptr,
- pdfium::WrapRetain(pStream));
+ auto form =
+ std::make_unique<CPDF_Form>(document(), nullptr, std::move(pStream));
CPDF_AllStates allStates;
allStates.m_ColorState.Emplace();
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index 4b3e4ff..224487f 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -360,7 +360,7 @@
CPDF_DictionaryLocker locker(this);
for (const auto& it : locker) {
const ByteString& key = it.first;
- CPDF_Object* pValue = it.second.Get();
+ const RetainPtr<CPDF_Object>& pValue = it.second;
if (!archive->WriteString("/") ||
!archive->WriteString(PDF_NameEncode(key).AsStringView())) {
return false;
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index 83e090e..4dcdcc8 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -239,14 +239,15 @@
if (m_pTreeTraversal.size() == level + 1)
m_pTreeTraversal.emplace_back(std::move(pKid), 0);
// Now m_pTreeTraversal[level+1] should exist and be equal to pKid.
- CPDF_Dictionary* pageKid = TraversePDFPages(iPage, nPagesToGo, level + 1);
+ RetainPtr<CPDF_Dictionary> pPageKid =
+ TraversePDFPages(iPage, nPagesToGo, level + 1);
// Check if child was completely processed, i.e. it popped itself out
if (m_pTreeTraversal.size() == level + 1)
m_pTreeTraversal[level].second++;
// If child did not finish, no pages to go, or max level reached, end
if (m_pTreeTraversal.size() != level + 1 || *nPagesToGo == 0 ||
m_bReachedMaxPageLevel) {
- page.Reset(pageKid);
+ page = std::move(pPageKid);
break;
}
}
@@ -350,7 +351,7 @@
bSkipped = true;
}
}
- const CPDF_Dictionary* pPages = GetPagesDict();
+ RetainPtr<const CPDF_Dictionary> pPages = GetPagesDict();
if (!pPages)
return -1;
diff --git a/core/fpdfapi/parser/cpdf_name.h b/core/fpdfapi/parser/cpdf_name.h
index d23ebd8..c3c023f 100644
--- a/core/fpdfapi/parser/cpdf_name.h
+++ b/core/fpdfapi/parser/cpdf_name.h
@@ -41,6 +41,10 @@
return obj ? obj->AsName() : nullptr;
}
+inline RetainPtr<const CPDF_Name> ToName(RetainPtr<CPDF_Object> obj) {
+ return RetainPtr<CPDF_Name>(ToName(obj.Get()));
+}
+
inline RetainPtr<const CPDF_Name> ToName(RetainPtr<const CPDF_Object> obj) {
return RetainPtr<const CPDF_Name>(ToName(obj.Get()));
}
diff --git a/core/fpdfapi/parser/fpdf_parser_utility.cpp b/core/fpdfapi/parser/fpdf_parser_utility.cpp
index d6110b8..950753e 100644
--- a/core/fpdfapi/parser/fpdf_parser_utility.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_utility.cpp
@@ -232,7 +232,7 @@
buf << "<<";
for (const auto& it : locker) {
const ByteString& key = it.first;
- const CPDF_Object* pValue = it.second.Get();
+ const RetainPtr<CPDF_Object>& pValue = it.second;
buf << "/" << PDF_NameEncode(key);
if (!pValue->IsInline()) {
buf << " " << pValue->GetObjNum() << " 0 R ";
diff --git a/core/fpdfdoc/cpdf_action.cpp b/core/fpdfdoc/cpdf_action.cpp
index ad915c0..1370c7f 100644
--- a/core/fpdfdoc/cpdf_action.cpp
+++ b/core/fpdfdoc/cpdf_action.cpp
@@ -147,14 +147,14 @@
}
absl::optional<WideString> CPDF_Action::MaybeGetJavaScript() const {
- const CPDF_Object* pObject = GetJavaScriptObject();
+ RetainPtr<const CPDF_Object> pObject = GetJavaScriptObject();
if (!pObject)
return absl::nullopt;
return pObject->GetUnicodeText();
}
WideString CPDF_Action::GetJavaScript() const {
- const CPDF_Object* pObject = GetJavaScriptObject();
+ RetainPtr<const CPDF_Object> pObject = GetJavaScriptObject();
return pObject ? pObject->GetUnicodeText() : WideString();
}
diff --git a/core/fpdfdoc/cpdf_filespec.cpp b/core/fpdfdoc/cpdf_filespec.cpp
index bb609c2..2e982b2 100644
--- a/core/fpdfdoc/cpdf_filespec.cpp
+++ b/core/fpdfdoc/cpdf_filespec.cpp
@@ -153,7 +153,7 @@
}
RetainPtr<const CPDF_Dictionary> CPDF_FileSpec::GetParamsDict() const {
- const CPDF_Stream* pStream = GetFileStream();
+ RetainPtr<const CPDF_Stream> pStream = GetFileStream();
if (!pStream)
return nullptr;
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index 3e38af5..bd6601c 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -40,7 +40,7 @@
if (!pFieldDict || nLevel > kGetFieldMaxRecursion)
return nullptr;
- const CPDF_Object* pAttr = pFieldDict->GetDirectObjectFor(name).Get();
+ RetainPtr<const CPDF_Object> pAttr = pFieldDict->GetDirectObjectFor(name);
if (pAttr)
return pAttr;
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index c09e233..8b16826 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -402,12 +402,11 @@
CPDF_DictionaryLocker locker(pSrcPageDict);
for (const auto& it : locker) {
const ByteString& cbSrcKeyStr = it.first;
+ const RetainPtr<CPDF_Object>& pObj = it.second;
if (cbSrcKeyStr == pdfium::page_object::kType ||
cbSrcKeyStr == pdfium::page_object::kParent) {
continue;
}
-
- CPDF_Object* pObj = it.second.Get();
pDestPageDict->SetFor(cbSrcKeyStr, pObj->Clone());
}
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 1585d5d..65b3c9d 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -372,7 +372,7 @@
if (!pDoc || !pDoc->GetParser())
return -1;
- const CPDF_Dictionary* pDict = pDoc->GetParser()->GetEncryptDict();
+ RetainPtr<const CPDF_Dictionary> pDict = pDoc->GetParser()->GetEncryptDict();
return pDict ? pDict->GetIntegerFor("R") : -1;
}
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp
index ca71694..ff93008 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp
@@ -764,21 +764,21 @@
return false;
}
- const CPDF_Array* pArray = ToArray(pAcroForm->GetObjectFor("XFA"));
+ RetainPtr<const CPDF_Array> pArray = ToArray(pAcroForm->GetObjectFor("XFA"));
if (!pArray) {
fileStream->Flush();
return false;
}
for (size_t i = 1; i < pArray->size(); i += 2) {
- const CPDF_Object* pPDFObj = pArray->GetObjectAt(i);
- const CPDF_Object* pPrePDFObj = pArray->GetObjectAt(i - 1);
+ RetainPtr<const CPDF_Object> pPDFObj = pArray->GetObjectAt(i);
+ RetainPtr<const CPDF_Object> pPrePDFObj = pArray->GetObjectAt(i - 1);
if (!pPrePDFObj->IsString())
continue;
if (!pPDFObj->IsReference())
continue;
- const CPDF_Object* pDirectObj = pPDFObj->GetDirect();
+ RetainPtr<const CPDF_Object> pDirectObj = pPDFObj->GetDirect();
if (!pDirectObj->IsStream())
continue;
ByteString bsType = pPrePDFObj->GetString();
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index c529158..5a842ce 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -699,7 +699,7 @@
CPDF_DictionaryLocker locker(ToDictionary(pDictionary->Clone()));
for (const auto& it : locker) {
const ByteString& bsKey = it.first;
- CPDF_Object* pValueObj = it.second.Get();
+ const RetainPtr<CPDF_Object>& pValueObj = it.second;
if (pValueObj->IsString() || pValueObj->IsName()) {
pRuntime->PutObjectProperty(
pObj, bsKey.AsStringView(),
diff --git a/xfa/fgas/font/cfgas_pdffontmgr.cpp b/xfa/fgas/font/cfgas_pdffontmgr.cpp
index 62ba9fa..c3efa1d 100644
--- a/xfa/fgas/font/cfgas_pdffontmgr.cpp
+++ b/xfa/fgas/font/cfgas_pdffontmgr.cpp
@@ -57,7 +57,7 @@
CPDF_DictionaryLocker locker(pFontSetDict);
for (const auto& it : locker) {
const ByteString& key = it.first;
- CPDF_Object* pObj = it.second.Get();
+ const RetainPtr<CPDF_Object>& pObj = it.second;
if (!PsNameMatchDRFontName(name.AsStringView(), bBold, bItalic, key,
bStrictMatch)) {
continue;
