From 0d830c1e0db684d17f9b3c534dec8cecb17b674d Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 7 Nov 2016 15:14:09 -0800 Subject: Hold trailers via unique_ptrs. Follow-up from ParseIndirectObject() CL. This will get cleaner once CPDF_Object::Clone itself returns unique_ptrs. Pack some bools together while we're at it. Review-Url: https://codereview.chromium.org/2479353002 --- core/fpdfapi/parser/cpdf_parser.cpp | 58 +++++++++++++++---------------------- core/fpdfapi/parser/cpdf_parser.h | 12 ++++---- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index 905f36622a..cf23ec7cd7 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp @@ -51,16 +51,15 @@ CPDF_Parser::CPDF_Parser() : m_pDocument(nullptr), m_bHasParsed(false), m_bOwnFileRead(true), + m_bXRefStream(false), + m_bVersionUpdated(false), m_FileVersion(0), - m_pTrailer(nullptr), m_pEncryptDict(nullptr), - m_bVersionUpdated(false), m_dwXrefStartObjNum(0) { m_pSyntax.reset(new CPDF_SyntaxParser); } CPDF_Parser::~CPDF_Parser() { - delete m_pTrailer; ReleaseEncryptHandler(); SetEncryptDictionary(nullptr); @@ -68,9 +67,6 @@ CPDF_Parser::~CPDF_Parser() { m_pSyntax->m_pFileAccess->Release(); m_pSyntax->m_pFileAccess = nullptr; } - - for (CPDF_Dictionary* trailer : m_Trailers) - delete trailer; } uint32_t CPDF_Parser::GetLastObjNum() const { @@ -323,7 +319,7 @@ bool CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { if (!m_pTrailer) return false; - int32_t xrefsize = GetDirectInteger(m_pTrailer, "Size"); + int32_t xrefsize = GetDirectInteger(m_pTrailer.get(), "Size"); if (xrefsize > 0 && xrefsize <= kMaxXRefSize) ShrinkObjectMap(xrefsize); @@ -332,12 +328,12 @@ bool CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { std::set seen_xrefpos; CrossRefList.push_back(xrefpos); - XRefStreamList.push_back(GetDirectInteger(m_pTrailer, "XRefStm")); + XRefStreamList.push_back(GetDirectInteger(m_pTrailer.get(), "XRefStm")); seen_xrefpos.insert(xrefpos); // When |m_pTrailer| doesn't have Prev entry or Prev entry value is not // numerical, GetDirectInteger() returns 0. Loading will end. - xrefpos = GetDirectInteger(m_pTrailer, "Prev"); + xrefpos = GetDirectInteger(m_pTrailer.get(), "Prev"); while (xrefpos) { // Check for circular references. if (pdfium::ContainsKey(seen_xrefpos, xrefpos)) @@ -358,7 +354,7 @@ bool CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { // SLOW ... XRefStreamList.insert(XRefStreamList.begin(), pDict->GetIntegerFor("XRefStm")); - m_Trailers.push_back(pDict.release()); + m_Trailers.push_back(std::move(pDict)); } for (size_t i = 0; i < CrossRefList.size(); ++i) { @@ -379,7 +375,7 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, if (!m_pTrailer) return false; - int32_t xrefsize = GetDirectInteger(m_pTrailer, "Size"); + int32_t xrefsize = GetDirectInteger(m_pTrailer.get(), "Size"); if (xrefsize == 0) return false; @@ -388,10 +384,10 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, std::set seen_xrefpos; CrossRefList.push_back(xrefpos); - XRefStreamList.push_back(GetDirectInteger(m_pTrailer, "XRefStm")); + XRefStreamList.push_back(GetDirectInteger(m_pTrailer.get(), "XRefStm")); seen_xrefpos.insert(xrefpos); - xrefpos = GetDirectInteger(m_pTrailer, "Prev"); + xrefpos = GetDirectInteger(m_pTrailer.get(), "Prev"); while (xrefpos) { // Check for circular references. if (pdfium::ContainsKey(seen_xrefpos, xrefpos)) @@ -412,7 +408,7 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, // SLOW ... XRefStreamList.insert(XRefStreamList.begin(), pDict->GetIntegerFor("XRefStm")); - m_Trailers.push_back(pDict.release()); + m_Trailers.push_back(std::move(pDict)); } for (size_t i = 1; i < CrossRefList.size(); ++i) { @@ -580,8 +576,7 @@ bool CPDF_Parser::LoadAllCrossRefV5(FX_FILESIZE xrefpos) { bool CPDF_Parser::RebuildCrossRef() { m_ObjectInfo.clear(); m_SortedOffset.clear(); - delete m_pTrailer; - m_pTrailer = nullptr; + m_pTrailer.reset(); ParserState state = ParserState::kDefault; int32_t inside_index = 0; @@ -742,8 +737,8 @@ bool CPDF_Parser::RebuildCrossRef() { CPDF_Object* pRoot = pDict->GetObjectFor("Root"); if (pRoot && pRoot->GetDict() && pRoot->GetDict()->GetObjectFor("Pages")) { - delete m_pTrailer; - m_pTrailer = ToDictionary(pDict->Clone()); + m_pTrailer = + ToDictionary(pdfium::WrapUnique(pDict->Clone())); } } } @@ -822,10 +817,10 @@ bool CPDF_Parser::RebuildCrossRef() { } } else { if (pObj->IsStream()) { - m_pTrailer = ToDictionary(pTrailer->Clone()); + m_pTrailer = + ToDictionary(pdfium::WrapUnique(pTrailer->Clone())); } else { - pObj.release(); - m_pTrailer = pTrailer; + m_pTrailer = ToDictionary(std::move(pObj)); } FX_FILESIZE dwSavePos = m_pSyntax->SavePos(); @@ -964,14 +959,15 @@ bool CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, bool bMainXRef) { if (size < 0) return false; - CPDF_Dictionary* pNewTrailer = ToDictionary(pDict->Clone()); + std::unique_ptr pNewTrailer = + ToDictionary(pdfium::WrapUnique(pDict->Clone())); if (bMainXRef) { - m_pTrailer = pNewTrailer; + m_pTrailer = std::move(pNewTrailer); ShrinkObjectMap(size); for (auto& it : m_ObjectInfo) it.second.type = 0; } else { - m_Trailers.push_back(pNewTrailer); + m_Trailers.push_back(std::move(pNewTrailer)); } std::vector> arrIndex; @@ -1400,15 +1396,11 @@ uint32_t CPDF_Parser::GetFirstPageNo() const { return m_pLinearized ? m_pLinearized->GetFirstPageNo() : 0; } -CPDF_Dictionary* CPDF_Parser::LoadTrailerV4() { +std::unique_ptr CPDF_Parser::LoadTrailerV4() { if (m_pSyntax->GetKeyword() != "trailer") return nullptr; - std::unique_ptr pObj( - m_pSyntax->GetObject(m_pDocument, 0, 0, true)); - if (!ToDictionary(pObj.get())) - return nullptr; - return pObj.release()->AsDictionary(); + return ToDictionary(m_pSyntax->GetObject(m_pDocument, 0, 0, true)); } uint32_t CPDF_Parser::GetPermissions() const { @@ -1494,7 +1486,7 @@ CPDF_Parser::Error CPDF_Parser::StartLinearizedParse( if (!m_pTrailer) return SUCCESS; - int32_t xrefsize = GetDirectInteger(m_pTrailer, "Size"); + int32_t xrefsize = GetDirectInteger(m_pTrailer.get(), "Size"); if (xrefsize > 0) ShrinkObjectMap(xrefsize); } @@ -1561,9 +1553,7 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV5(FX_FILESIZE xrefpos) { CPDF_Parser::Error CPDF_Parser::LoadLinearizedMainXRefTable() { uint32_t dwSaveMetadataObjnum = m_pSyntax->m_MetadataObjnum; m_pSyntax->m_MetadataObjnum = 0; - - delete m_pTrailer; - m_pTrailer = nullptr; + m_pTrailer.reset(); m_pSyntax->RestorePos(m_LastXRefOffset - m_pSyntax->m_HeaderOffset); uint8_t ch = 0; diff --git a/core/fpdfapi/parser/cpdf_parser.h b/core/fpdfapi/parser/cpdf_parser.h index 007193b3ea..5e2cdea08d 100644 --- a/core/fpdfapi/parser/cpdf_parser.h +++ b/core/fpdfapi/parser/cpdf_parser.h @@ -49,7 +49,7 @@ class CPDF_Parser { void SetPassword(const FX_CHAR* password) { m_Password = password; } CFX_ByteString GetPassword() { return m_Password; } - CPDF_Dictionary* GetTrailer() const { return m_pTrailer; } + CPDF_Dictionary* GetTrailer() const { return m_pTrailer.get(); } FX_FILESIZE GetLastXRefOffset() const { return m_LastXRefOffset; } uint32_t GetPermissions() const; @@ -132,7 +132,7 @@ class CPDF_Parser { bool LoadAllCrossRefV4(FX_FILESIZE pos); bool LoadAllCrossRefV5(FX_FILESIZE pos); bool LoadCrossRefV5(FX_FILESIZE* pos, bool bMainXRef); - CPDF_Dictionary* LoadTrailerV4(); + std::unique_ptr LoadTrailerV4(); Error SetEncryptHandler(); void ReleaseEncryptHandler(); bool LoadLinearizedAllCrossRefV4(FX_FILESIZE pos, uint32_t dwObjCount); @@ -150,16 +150,16 @@ class CPDF_Parser { CPDF_Document* m_pDocument; // not owned bool m_bHasParsed; bool m_bOwnFileRead; + bool m_bXRefStream; + bool m_bVersionUpdated; int m_FileVersion; - CPDF_Dictionary* m_pTrailer; CPDF_Dictionary* m_pEncryptDict; FX_FILESIZE m_LastXRefOffset; - bool m_bXRefStream; std::unique_ptr m_pSecurityHandler; CFX_ByteString m_Password; std::set m_SortedOffset; - std::vector m_Trailers; - bool m_bVersionUpdated; + std::unique_ptr m_pTrailer; + std::vector> m_Trailers; std::unique_ptr m_pLinearized; uint32_t m_dwXrefStartObjNum; -- cgit v1.2.3