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;