From 64f4e25304dfd93651ac5c9d5379ed2fffbb993f Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 7 Dec 2016 19:01:56 -0800 Subject: Use unique_ptr for CXFA_XMLParser. Also rename CFDE_XMLParser to IFDE_XMLParser since its an interface. Review-Url: https://codereview.chromium.org/2555373002 --- testing/libfuzzer/pdf_xml_fuzzer.cc | 9 +++------ xfa/fde/xml/fde_xml_imp.cpp | 19 +++++++++---------- xfa/fde/xml/fde_xml_imp.h | 14 +++++++------- xfa/fxfa/parser/cxfa_simple_parser.cpp | 18 +++++++++++------- xfa/fxfa/parser/cxfa_xml_parser.cpp | 4 ---- xfa/fxfa/parser/cxfa_xml_parser.h | 5 ++--- 6 files changed, 32 insertions(+), 37 deletions(-) diff --git a/testing/libfuzzer/pdf_xml_fuzzer.cc b/testing/libfuzzer/pdf_xml_fuzzer.cc index 4f000ab78e..e3cd254344 100644 --- a/testing/libfuzzer/pdf_xml_fuzzer.cc +++ b/testing/libfuzzer/pdf_xml_fuzzer.cc @@ -56,14 +56,11 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { if (!stream) return 0; - std::unique_ptr doc = pdfium::MakeUnique(); - std::unique_ptr> parser( - new CXFA_XMLParser(doc->GetRoot(), stream)); - if (!doc->LoadXML(parser.release())) + auto doc = pdfium::MakeUnique(); + if (!doc->LoadXML(pdfium::MakeUnique(doc->GetRoot(), stream))) return 0; - int32_t load_result = doc->DoLoad(nullptr); - if (load_result < 100) + if (doc->DoLoad(nullptr) < 100) return 0; (void)XFA_FDEExtension_GetDocumentNode(doc.get()); diff --git a/xfa/fde/xml/fde_xml_imp.cpp b/xfa/fde/xml/fde_xml_imp.cpp index f333b4fcda..7880804367 100644 --- a/xfa/fde/xml/fde_xml_imp.cpp +++ b/xfa/fde/xml/fde_xml_imp.cpp @@ -7,6 +7,7 @@ #include "xfa/fde/xml/fde_xml_imp.h" #include +#include #include "core/fxcrt/fx_ext.h" #include "core/fxcrt/fx_safe_types.h" @@ -957,29 +958,27 @@ void CFDE_XMLDoc::Reset(bool bInitRoot) { } void CFDE_XMLDoc::ReleaseParser() { - if (m_pXMLParser) { - m_pXMLParser->Release(); - m_pXMLParser = nullptr; - } + m_pXMLParser.reset(); if (m_pSyntaxParser) { m_pSyntaxParser->Release(); m_pSyntaxParser = nullptr; } } -bool CFDE_XMLDoc::LoadXML(CFDE_XMLParser* pXMLParser) { +bool CFDE_XMLDoc::LoadXML(std::unique_ptr pXMLParser) { if (!pXMLParser) return false; Reset(true); - m_pXMLParser = pXMLParser; - return !!m_pXMLParser; + m_pXMLParser = std::move(pXMLParser); + return true; } int32_t CFDE_XMLDoc::DoLoad(IFX_Pause* pPause) { - if (m_iStatus >= 100) - return m_iStatus; - return m_iStatus = m_pXMLParser->DoParser(pPause); + if (m_iStatus < 100) + m_iStatus = m_pXMLParser->DoParser(pPause); + + return m_iStatus; } void CFDE_XMLDoc::CloseXML() { diff --git a/xfa/fde/xml/fde_xml_imp.h b/xfa/fde/xml/fde_xml_imp.h index 283184aadc..8571f1b9f1 100644 --- a/xfa/fde/xml/fde_xml_imp.h +++ b/xfa/fde/xml/fde_xml_imp.h @@ -7,6 +7,8 @@ #ifndef XFA_FDE_XML_FDE_XML_IMP_H_ #define XFA_FDE_XML_FDE_XML_IMP_H_ +#include + #include "core/fxcrt/fx_system.h" #include "xfa/fde/xml/fde_xml.h" #include "xfa/fgas/crt/fgas_memory.h" @@ -19,8 +21,8 @@ class CFDE_XMLElement; class CFDE_XMLText; class CFDE_XMLDoc; class CFDE_XMLDOMParser; -class CFDE_XMLParser; class CFDE_XMLSyntaxParser; +class IFDE_XMLParser; class CFDE_XMLNode : public CFX_Target { public: @@ -193,7 +195,7 @@ class CFDE_XMLDoc : public CFX_Target { CFDE_XMLDoc(); ~CFDE_XMLDoc() override; - bool LoadXML(CFDE_XMLParser* pXMLParser); + bool LoadXML(std::unique_ptr pXMLParser); int32_t DoLoad(IFX_Pause* pPause = nullptr); void CloseXML(); CFDE_XMLNode* GetRoot() const { return m_pRoot; } @@ -209,14 +211,12 @@ class CFDE_XMLDoc : public CFX_Target { int32_t m_iStatus; CFDE_XMLNode* m_pRoot; CFDE_XMLSyntaxParser* m_pSyntaxParser; - CFDE_XMLParser* m_pXMLParser; + std::unique_ptr m_pXMLParser; }; -class CFDE_XMLParser { +class IFDE_XMLParser { public: - virtual ~CFDE_XMLParser() {} - - virtual void Release() = 0; + virtual ~IFDE_XMLParser() {} virtual int32_t DoParser(IFX_Pause* pPause) = 0; }; diff --git a/xfa/fxfa/parser/cxfa_simple_parser.cpp b/xfa/fxfa/parser/cxfa_simple_parser.cpp index 9eca652db7..9f6fef9c1f 100644 --- a/xfa/fxfa/parser/cxfa_simple_parser.cpp +++ b/xfa/fxfa/parser/cxfa_simple_parser.cpp @@ -6,6 +6,8 @@ #include "xfa/fxfa/parser/cxfa_simple_parser.h" +#include + #include "core/fxcrt/fx_ext.h" #include "third_party/base/ptr_util.h" #include "xfa/fgas/crt/fgas_codepage.h" @@ -293,9 +295,11 @@ int32_t CXFA_SimpleParser::StartParse( wCodePage != FX_CODEPAGE_UTF8) { m_pStream->SetCodePage(FX_CODEPAGE_UTF8); } - m_pXMLDoc.reset(new CFDE_XMLDoc); - m_pXMLParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream); - if (!m_pXMLDoc->LoadXML(m_pXMLParser)) + m_pXMLDoc = pdfium::MakeUnique(); + auto pNewParser = + pdfium::MakeUnique(m_pXMLDoc->GetRoot(), m_pStream); + m_pXMLParser = pNewParser.get(); + if (!m_pXMLDoc->LoadXML(std::move(pNewParser))) return XFA_PARSESTATUS_StatusErr; m_ePacketID = ePacketID; @@ -326,12 +330,12 @@ int32_t CXFA_SimpleParser::ParseXMLData(const CFX_WideString& wsXML, IFX_Pause* pPause) { CloseParser(); pXMLNode = nullptr; - - auto pStream = pdfium::MakeRetain(wsXML); m_pXMLDoc = pdfium::MakeUnique(); - CXFA_XMLParser* pParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), pStream); + auto pStream = pdfium::MakeRetain(wsXML); + auto pParser = + pdfium::MakeUnique(m_pXMLDoc->GetRoot(), pStream); pParser->m_dwCheckStatus = 0x03; - if (!m_pXMLDoc->LoadXML(pParser)) + if (!m_pXMLDoc->LoadXML(std::move(pParser))) return XFA_PARSESTATUS_StatusErr; int32_t iRet = m_pXMLDoc->DoLoad(pPause); diff --git a/xfa/fxfa/parser/cxfa_xml_parser.cpp b/xfa/fxfa/parser/cxfa_xml_parser.cpp index 6d2ef350d9..95920d9773 100644 --- a/xfa/fxfa/parser/cxfa_xml_parser.cpp +++ b/xfa/fxfa/parser/cxfa_xml_parser.cpp @@ -29,10 +29,6 @@ CXFA_XMLParser::~CXFA_XMLParser() { m_ws2.clear(); } -void CXFA_XMLParser::Release() { - delete this; -} - int32_t CXFA_XMLParser::DoParser(IFX_Pause* pPause) { if (m_syntaxParserResult == FDE_XmlSyntaxResult::Error) return -1; diff --git a/xfa/fxfa/parser/cxfa_xml_parser.h b/xfa/fxfa/parser/cxfa_xml_parser.h index 4c64ee2f8c..e49a9dd127 100644 --- a/xfa/fxfa/parser/cxfa_xml_parser.h +++ b/xfa/fxfa/parser/cxfa_xml_parser.h @@ -14,14 +14,13 @@ class IFGAS_Stream; class IFX_Pause; -class CXFA_XMLParser : public CFDE_XMLParser { +class CXFA_XMLParser : public IFDE_XMLParser { public: CXFA_XMLParser(CFDE_XMLNode* pRoot, const CFX_RetainPtr& pStream); ~CXFA_XMLParser() override; - // CFDE_XMLParser - void Release() override; + // IFDE_XMLParser int32_t DoParser(IFX_Pause* pPause) override; FX_FILESIZE m_nStart[2]; -- cgit v1.2.3