loplugin:useuniqueptr in xmlsecurity::BufferNode

Change-Id: Ifaea9f2f0ef5b84372b2ba51deacdb4149bb66e1
Reviewed-on: https://gerrit.libreoffice.org/60616
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

xmlsecurity/source/framework/buffernode.cxx

@@ -278,9 +278,14 @@ bool BufferNode::hasChildren() const
     return (!m_vChildren.empty());
 }
 
-std::vector< const BufferNode* >* BufferNode::getChildren() const
+std::vector< std::unique_ptr<BufferNode> > const & BufferNode::getChildren() const
 {
-    return new std::vector< const BufferNode* >( m_vChildren );
+    return m_vChildren;
+}
+
+std::vector< std::unique_ptr<BufferNode> > BufferNode::releaseChildren()
+{
+    return std::move(m_vChildren);
 }
 
 const BufferNode* BufferNode::getFirstChild() const
@@ -307,13 +312,13 @@ const BufferNode* BufferNode::getFirstChild() const
 
     if (!m_vChildren.empty())
     {
-        rc = const_cast<BufferNode*>(m_vChildren.front());
+        rc = m_vChildren.front().get();
     }
 
     return rc;
 }
 
-void BufferNode::addChild(const BufferNode* pChild, sal_Int32 nPosition)
+void BufferNode::addChild(std::unique_ptr<BufferNode> pChild, sal_Int32 nPosition)
 /****** BufferNode/addChild(pChild,nPosition) ********************************
  *
  *   NAME
@@ -339,17 +344,15 @@ void BufferNode::addChild(const BufferNode* pChild, sal_Int32 nPosition)
 {
     if (nPosition == -1)
     {
-        m_vChildren.push_back( pChild );
+        m_vChildren.push_back( std::move(pChild) );
     }
     else
     {
-        std::vector< const BufferNode* >::iterator ii = m_vChildren.begin();
-        ii += nPosition;
-        m_vChildren.insert(ii, pChild);
+        m_vChildren.insert(m_vChildren.begin() + nPosition, std::move(pChild));
     }
 }
 
-void BufferNode::addChild(const BufferNode* pChild)
+void BufferNode::addChild(std::unique_ptr<BufferNode> pChild)
 /****** BufferNode/addChild() ************************************************
  *
  *   NAME
@@ -371,14 +374,14 @@ void BufferNode::addChild(const BufferNode* pChild)
  *  The new child BufferNode is appended at the end.
  ******************************************************************************/
 {
-    addChild(pChild, -1);
+    addChild(std::move(pChild), -1);
 }
 
 void BufferNode::removeChild(const BufferNode* pChild)
 /****** BufferNode/removeChild ***********************************************
  *
  *   NAME
- *  removeChild -- removes a child BufferNode from the children list.
+ *  removeChild -- removes and deletes a child BufferNode from the children list.
  *
  *   SYNOPSIS
  *  removeChild(pChild);
@@ -393,7 +396,9 @@ void BufferNode::removeChild(const BufferNode* pChild)
  *  empty
  ******************************************************************************/
 {
-    auto ii = std::find(m_vChildren.begin(), m_vChildren.end(), pChild);
+    auto ii = std::find_if(m_vChildren.begin(), m_vChildren.end(),
+                [pChild] (const std::unique_ptr<BufferNode>& i)
+                { return i.get() == pChild; });
     if (ii != m_vChildren.end())
         m_vChildren.erase( ii );
 }
@@ -418,7 +423,9 @@ sal_Int32 BufferNode::indexOfChild(const BufferNode* pChild) const
  *          is not found, -1 is returned.
  ******************************************************************************/
 {
-    auto ii = std::find(m_vChildren.begin(), m_vChildren.end(), pChild);
+    auto ii = std::find_if(m_vChildren.begin(), m_vChildren.end(),
+            [pChild] (const std::unique_ptr<BufferNode>& i)
+            { return i.get() == pChild; });
     if (ii == m_vChildren.end())
         return -1;
 
@@ -486,12 +493,12 @@ const BufferNode* BufferNode::isAncestor(const BufferNode* pDescendant) const
     if (pDescendant != nullptr)
     {
         auto ii = std::find_if(m_vChildren.cbegin(), m_vChildren.cend(),
-            [&pDescendant](const BufferNode* pChild) {
-                return (pChild == pDescendant) || (pChild->isAncestor(pDescendant) != nullptr);
+            [&pDescendant](const std::unique_ptr<BufferNode>& pChild) {
+                return (pChild.get() == pDescendant) || (pChild->isAncestor(pDescendant) != nullptr);
             });
 
         if (ii != m_vChildren.end())
-            rc = const_cast<BufferNode*>(*ii);
+            rc = ii->get();
     }
 
     return rc;
@@ -636,9 +643,8 @@ void BufferNode::notifyBranch()
  *  empty
  ******************************************************************************/
 {
-    for( const BufferNode* ii : m_vChildren )
+    for( std::unique_ptr<BufferNode>& pBufferNode : m_vChildren )
     {
-        BufferNode* pBufferNode = const_cast<BufferNode*>(ii);
         pBufferNode->elementCollectorNotify();
         pBufferNode->notifyBranch();
     }
@@ -756,7 +762,7 @@ bool BufferNode::isECInSubTreeIncluded(sal_Int32 nIgnoredSecurityId) const
     if ( !rc )
     {
         rc = std::any_of(m_vChildren.begin(), m_vChildren.end(),
-            [nIgnoredSecurityId](const BufferNode* pBufferNode) {
+            [nIgnoredSecurityId](const std::unique_ptr<BufferNode>& pBufferNode) {
                 return pBufferNode->isECInSubTreeIncluded(nIgnoredSecurityId);
         });
     }
@@ -832,7 +838,7 @@ bool BufferNode::isBlockerInSubTreeIncluded(sal_Int32 nIgnoredSecurityId) const
  ******************************************************************************/
 {
     return std::any_of(m_vChildren.begin(), m_vChildren.end(),
-        [nIgnoredSecurityId](const BufferNode* pBufferNode) {
+        [nIgnoredSecurityId](const std::unique_ptr<BufferNode>& pBufferNode) {
             ElementMark* pBlocker = pBufferNode->getBlocker();
             return (pBlocker != nullptr &&
                 (nIgnoredSecurityId == cssxc::sax::ConstOfSecurityId::UNDEFINEDSECURITYID ||
@@ -864,15 +870,15 @@ const BufferNode* BufferNode::getNextChild(const BufferNode* pChild) const
     BufferNode* rc = nullptr;
     bool bChildFound = false;
 
-    for( const BufferNode* i : m_vChildren )
+    for( std::unique_ptr<BufferNode> const & i : m_vChildren )
     {
         if (bChildFound)
         {
-            rc = const_cast<BufferNode*>(i);
+            rc = i.get();
             break;
         }
 
-        if( i == pChild )
+        if( i.get() == pChild )
         {
             bChildFound = true;
         }
@@ -881,34 +887,4 @@ const BufferNode* BufferNode::getNextChild(const BufferNode* pChild) const
     return rc;
 }
 
-
-void BufferNode::freeAllChildren()
-/****** BufferNode/freeAllChildren *******************************************
- *
- *   NAME
- *  freeAllChildren -- free all his child BufferNode.
- *
- *   SYNOPSIS
- *  freeAllChildren();
- *
- *   FUNCTION
- *  see NAME
- *
- *   INPUTS
- *  empty
- *
- *   RESULT
- *  empty
- ******************************************************************************/
-{
-    for( const BufferNode* i : m_vChildren )
-    {
-        BufferNode *pChild = const_cast<BufferNode *>(i);
-        pChild->freeAllChildren();
-        delete pChild;
-    }
-
-    m_vChildren.clear();
-}
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

xmlsecurity/source/framework/buffernode.hxx

@@ -23,6 +23,7 @@
 #include <com/sun/star/lang/XServiceInfo.hpp>
 #include <com/sun/star/xml/wrapper/XXMLElementWrapper.hpp>
 
+#include <memory>
 #include <vector>
 
 class ElementMark;
@@ -49,7 +50,7 @@ private:
     BufferNode* m_pParent;
 
     /* all child BufferNodes */
-    std::vector< const BufferNode* > m_vChildren;
+    std::vector< std::unique_ptr<BufferNode> > m_vChildren;
 
     /* all ElementCollector holding this BufferNode */
     std::vector< const ElementCollector* > m_vElementCollectors;
@@ -89,10 +90,11 @@ public:
     OUString printChildren() const;
     bool hasAnything() const;
     bool hasChildren() const;
-    std::vector< const BufferNode* >* getChildren() const;
+    std::vector< std::unique_ptr< BufferNode> > const & getChildren() const;
+    std::vector< std::unique_ptr< BufferNode> > releaseChildren();
     const BufferNode* getFirstChild() const;
-    void addChild(const BufferNode* pChild, sal_Int32 nPosition);
-    void addChild(const BufferNode* pChild);
+    void addChild(std::unique_ptr<BufferNode> pChild, sal_Int32 nPosition);
+    void addChild(std::unique_ptr<BufferNode> pChild);
     void removeChild(const BufferNode* pChild);
     sal_Int32 indexOfChild(const BufferNode* pChild) const;
     const BufferNode* getParent() const { return m_pParent;}
@@ -106,7 +108,6 @@ public:
         css::xml::wrapper::XXMLElementWrapper >& xXMLElement);
     void notifyBranch();
     void elementCollectorNotify();
-    void freeAllChildren();
 };
 
 #endif

xmlsecurity/source/framework/saxeventkeeperimpl.cxx

@@ -56,11 +56,7 @@ SAXEventKeeperImpl::~SAXEventKeeperImpl()
     /*
      * delete the BufferNode tree
      */
-    if (m_pRootBufferNode != nullptr)
-    {
-        m_pRootBufferNode->freeAllChildren();
-        m_pRootBufferNode.reset();
-    }
+    m_pRootBufferNode.reset();
 
     m_pCurrentBufferNode = m_pCurrentBlockingBufferNode = nullptr;
 
@@ -103,7 +99,7 @@ void SAXEventKeeperImpl::setCurrentBufferNode(BufferNode* pBufferNode)
 
         if (pBufferNode->getParent() == nullptr)
         {
-            m_pCurrentBufferNode->addChild(pBufferNode);
+            m_pCurrentBufferNode->addChild(std::unique_ptr<BufferNode>(pBufferNode));
             pBufferNode->setParent(m_pCurrentBufferNode);
         }
 
@@ -329,14 +325,12 @@ OUString SAXEventKeeperImpl::printBufferNode(
     }
     rc.append("\n");
 
-    std::vector< const BufferNode* >* vChildren = pBufferNode->getChildren();
-    for( const auto& jj : *vChildren )
+    std::vector< std::unique_ptr<BufferNode> > const & vChildren = pBufferNode->getChildren();
+    for( const auto& jj : vChildren )
     {
-        rc.append(printBufferNode(jj, nIndent+4));
+        rc.append(printBufferNode(jj.get(), nIndent+4));
     }
 
-    delete vChildren;
-
     return rc.makeStringAndClear();
 }
 
@@ -358,20 +352,18 @@ cssu::Sequence< cssu::Reference< cssxw::XXMLElementWrapper > >
  *  list - the child Elements list.
  ******************************************************************************/
 {
-    std::vector< const BufferNode* >* vChildren = pBufferNode->getChildren();
+    std::vector< std::unique_ptr<BufferNode> > const & vChildren = pBufferNode->getChildren();
 
     cssu::Sequence < cssu::Reference<
-        cssxw::XXMLElementWrapper > > aChildrenCollection ( vChildren->size());
+        cssxw::XXMLElementWrapper > > aChildrenCollection ( vChildren.size());
 
     sal_Int32 nIndex = 0;
-    for( const auto& i : *vChildren )
+    for( const auto& i : vChildren )
     {
         aChildrenCollection[nIndex] = i->getXMLElement();
         nIndex++;
     }
 
-    delete vChildren;
-
     return aChildrenCollection;
 }
 
@@ -501,23 +493,16 @@ void SAXEventKeeperImpl::smashBufferNode(
 
         sal_Int32 nIndex = pParent->indexOfChild(pBufferNode);
 
-        std::vector< const BufferNode* >* vChildren = pBufferNode->getChildren();
-        pParent->removeChild(pBufferNode);
-        pBufferNode->setParent(nullptr);
+        std::vector< std::unique_ptr<BufferNode> > vChildren = pBufferNode->releaseChildren();
+        pParent->removeChild(pBufferNode); // delete buffernode
 
-        for( const auto& i : *vChildren )
+        for( auto& i : vChildren )
         {
-            const_cast<BufferNode *>(i)->setParent(pParent);
-            pParent->addChild(i, nIndex);
+            i->setParent(pParent);
+            pParent->addChild(std::move(i), nIndex);
             nIndex++;
         }
 
-        delete vChildren;
-
-        /*
-         * delete the BufferNode
-         */
-        delete pBufferNode;
     }
 }