Cleanup ownership of parser members

xfa/fxfa/parser/cxfa_simple_parser.cpp

 // Copyright 2016 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "xfa/fxfa/parser/cxfa_simple_parser.h"
 
 #include "xfa/fgas/crt/fgas_codepage.h"
 #include "xfa/fxfa/include/fxfa.h"
 #include "xfa/fxfa/include/xfa_checksum.h"
 #include "xfa/fxfa/parser/cxfa_xml_parser.h"
 #include "xfa/fxfa/parser/xfa_document.h"
 
 CXFA_SimpleParser::CXFA_SimpleParser(CXFA_Document* pFactory,
                                      bool bDocumentParser)
     : m_pXMLParser(nullptr),
       m_pXMLDoc(nullptr),
       m_pStream(nullptr),
       m_pFileRead(nullptr),
       m_pFactory(pFactory),
       m_pRootNode(nullptr),
       m_ePacketID(XFA_XDPPACKET_UNKNOWN),
       m_bDocumentParser(bDocumentParser) {}
 
 CXFA_SimpleParser::~CXFA_SimpleParser() {
-  CloseParser();
 }
 
 void CXFA_SimpleParser::SetFactory(CXFA_Document* pFactory) {
   m_pFactory = pFactory;
 }
 
 static CFDE_XMLNode* XFA_FDEExtension_GetDocumentNode(
     CFDE_XMLDoc* pXMLDoc,
     FX_BOOL bVerifyWellFormness = FALSE) {
   if (!pXMLDoc) {
@@ skipping 16 matching lines @@
       }
       return pXMLNode;
     }
   }
   return nullptr;
 }
 int32_t CXFA_SimpleParser::StartParse(IFX_FileRead* pStream,
                                       XFA_XDPPACKET ePacketID) {
   CloseParser();
   m_pFileRead = pStream;
-  m_pStream = IFX_Stream::CreateStream(
-      pStream, FX_STREAMACCESS_Read | FX_STREAMACCESS_Text);
-  if (!m_pStream) {
+  m_pStream.reset(IFX_Stream::CreateStream(
+      pStream, FX_STREAMACCESS_Read | FX_STREAMACCESS_Text));
+  if (!m_pStream)
     return XFA_PARSESTATUS_StreamErr;
-  }
+
   uint16_t wCodePage = m_pStream->GetCodePage();
   if (wCodePage != FX_CODEPAGE_UTF16LE && wCodePage != FX_CODEPAGE_UTF16BE &&
       wCodePage != FX_CODEPAGE_UTF8) {
     m_pStream->SetCodePage(FX_CODEPAGE_UTF8);
   }
-  m_pXMLDoc = new CFDE_XMLDoc;
-  m_pXMLParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream);
-  if (!m_pXMLDoc->LoadXML(m_pXMLParser)) {
+  m_pXMLDoc.reset(new CFDE_XMLDoc);
+  m_pXMLParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream.get());
+  if (!m_pXMLDoc->LoadXML(m_pXMLParser))
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   m_ePacketID = ePacketID;
   return XFA_PARSESTATUS_Ready;
 }

[Wei Li, 2016/07/07 22:22:11]

Blank line here? Ditto.

[dsinclair, 2016/07/11 13:41:29]

Done.

 int32_t CXFA_SimpleParser::DoParse(IFX_Pause* pPause) {
-  if (!m_pXMLDoc || m_ePacketID == XFA_XDPPACKET_UNKNOWN) {
+  if (!m_pXMLDoc || m_ePacketID == XFA_XDPPACKET_UNKNOWN)
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   int32_t iRet = m_pXMLDoc->DoLoad(pPause);
-  if (iRet < 0) {
+  if (iRet < 0)
     return XFA_PARSESTATUS_SyntaxErr;
-  }
-  if (iRet < 100) {
+  if (iRet < 100)
     return iRet / 2;
-  }
-  m_pRootNode = ParseAsXDPPacket(XFA_FDEExtension_GetDocumentNode(m_pXMLDoc),
-                                 m_ePacketID);
+
+  m_pRootNode = ParseAsXDPPacket(
+      XFA_FDEExtension_GetDocumentNode(m_pXMLDoc.get()), m_ePacketID);
   m_pXMLDoc->CloseXML();
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
-  if (!m_pRootNode) {
+  m_pStream.reset();
+
+  if (!m_pRootNode)
     return XFA_PARSESTATUS_StatusErr;
-  }
   return XFA_PARSESTATUS_Done;
 }
 int32_t CXFA_SimpleParser::ParseXMLData(const CFX_WideString& wsXML,
                                         CFDE_XMLNode*& pXMLNode,
                                         IFX_Pause* pPause) {
   CloseParser();
   pXMLNode = nullptr;
   IFX_Stream* pStream = XFA_CreateWideTextRead(wsXML);
-  if (!pStream) {
+  if (!pStream)
     return XFA_PARSESTATUS_StreamErr;
-  }
-  m_pStream = pStream;
-  m_pXMLDoc = new CFDE_XMLDoc;
-  CXFA_XMLParser* pParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream);
+
+  m_pStream.reset(pStream);

[Wei Li, 2016/07/07 22:22:11]

Is |m_pStream| needed here? Having a unique_ptr of |pStream| may be simpler?

[dsinclair, 2016/07/11 13:41:29]

Done.

+  m_pXMLDoc.reset(new CFDE_XMLDoc);
+  CXFA_XMLParser* pParser =
+      new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream.get());
   pParser->m_dwCheckStatus = 0x03;
-  if (!m_pXMLDoc->LoadXML(pParser)) {
+  if (!m_pXMLDoc->LoadXML(pParser))
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   int32_t iRet = m_pXMLDoc->DoLoad(pPause);
-  if (iRet < 0 || iRet >= 100) {
+  if (iRet < 0 || iRet >= 100)
     m_pXMLDoc->CloseXML();
-  }
-  if (iRet < 0) {
+  if (iRet < 0)
     return XFA_PARSESTATUS_SyntaxErr;
-  }
-  if (iRet < 100) {
+  if (iRet < 100)
     return iRet / 2;
-  }
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
-  pXMLNode = XFA_FDEExtension_GetDocumentNode(m_pXMLDoc);
+
+  m_pStream.reset();
+
+  pXMLNode = XFA_FDEExtension_GetDocumentNode(m_pXMLDoc.get());
   return XFA_PARSESTATUS_Done;
 }
 
 void CXFA_SimpleParser::ConstructXFANode(CXFA_Node* pXFANode,
                                          CFDE_XMLNode* pXMLNode) {
   XFA_XDPPACKET ePacketID = (XFA_XDPPACKET)pXFANode->GetPacketID();
   if (ePacketID == XFA_XDPPACKET_Datasets) {
     if (pXFANode->GetElementType() == XFA_Element::DataValue) {
       for (CFDE_XMLNode* pXMLChild =
                pXMLNode->GetNodeItem(CFDE_XMLNode::FirstChild);
@@ skipping 34 matching lines @@
   } else {
     m_pRootNode = NormalLoader(pXFANode, pXMLNode, ePacketID);
   }
 }
 
 CXFA_Node* CXFA_SimpleParser::GetRootNode() const {
   return m_pRootNode;
 }
 
 CFDE_XMLDoc* CXFA_SimpleParser::GetXMLDoc() const {
-  return m_pXMLDoc;
+  return m_pXMLDoc.get();
 }
 
 FX_BOOL XFA_FDEExtension_ResolveNamespaceQualifier(
     CFDE_XMLElement* pNode,
     const CFX_WideStringC& wsQualifier,
     CFX_WideString& wsNamespaceURI) {
   if (!pNode) {
     return FALSE;
   }
   CFDE_XMLNode* pFakeRoot = pNode->GetNodeItem(CFDE_XMLNode::Root);
@@ skipping 1099 matching lines @@
     if (pXMLInstruction->GetData(0, wsData) &&
         wsData == FX_WSTRC(L"JavaScript")) {
       if (pXMLInstruction->GetData(1, wsData) &&
           wsData == FX_WSTRC(L"strictScoping")) {
         pXFANode->GetDocument()->SetFlag(XFA_DOCFLAG_StrictScoping, TRUE);
       }
     }
   }
 }
 void CXFA_SimpleParser::CloseParser() {
-  if (m_pXMLDoc) {
-    m_pXMLDoc->Release();
-    m_pXMLDoc = nullptr;
-  }
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
+  m_pXMLDoc.reset();
+  m_pStream.reset();
 }