xmlfix3: unoxml: fix various issues in CElementList:

 the string buffer allocated in ctor missed null terminator.
 the xmlChar* members of ElementList are leaked.
 the CElement* pointer does not keep the document alive.
 CDocument::getElementsByTagNameNS calls CElementList ctor with wrong args.

unoxml/source/dom/document.cxx

@@ -54,6 +54,20 @@
 
 namespace DOM
 {
+    /// get the pointer to the root element node of the document
+    static xmlNodePtr lcl_getDocumentRootPtr(xmlDocPtr const i_pDocument)
+    {
+        // find the document element
+        xmlNodePtr cur = i_pDocument->children;
+        while (cur != NULL)
+        {
+            if (cur->type == XML_ELEMENT_NODE)
+                break;
+            cur = cur->next;
+        }
+        return cur;
+    }
+
     CDocument::~CDocument()
     {
         xmlFreeDoc(m_aDocPtr);
@@ -73,6 +87,14 @@ namespace DOM
         return *m_pEventDispatcher;
     }
 
+    ::rtl::Reference< CElement > CDocument::GetDocumentElement()
+    {
+        xmlNodePtr const pNode = lcl_getDocumentRootPtr(m_aDocPtr);
+        ::rtl::Reference< CElement > const xRet(
+            dynamic_cast<CElement*>(CNode::getCNode(pNode).get()));
+        return xRet;
+    }
+
 
     void SAL_CALL CDocument::saxify(
             const Reference< XDocumentHandler >& i_xHandler) {
@@ -385,25 +407,12 @@ namespace DOM
         return xRet;
     }
 
-    /// get the pointer to the root element node of the document
-    static xmlNodePtr SAL_CALL _getDocumentRootPtr(xmlDocPtr i_pDocument) {
-        // find the document element
-        xmlNodePtr cur = i_pDocument->children;
-        while (cur != NULL)
-        {
-            if (cur->type == XML_ELEMENT_NODE)
-                break;
-            cur = cur->next;
-        }
-        return cur;
-    }
-
     // This is a convenience attribute that allows direct access to the child
     // node that is the root element of the document.
     Reference< XElement > SAL_CALL CDocument::getDocumentElement()
         throw (RuntimeException)
     {
-        xmlNodePtr const pNode = _getDocumentRootPtr(m_aDocPtr);
+        xmlNodePtr const pNode = lcl_getDocumentRootPtr(m_aDocPtr);
         Reference< XElement > const xRet(
             static_cast< XNode* >(CNode::getCNode(pNode).get()),
             UNO_QUERY_THROW);
@@ -452,22 +461,23 @@ namespace DOM
     }
 
 
-    Reference< XNodeList > SAL_CALL CDocument::getElementsByTagName(const OUString& tagname)
+    Reference< XNodeList > SAL_CALL
+    CDocument::getElementsByTagName(OUString const& rTagname)
             throw (RuntimeException)
     {
-        // build a list
-        return Reference< XNodeList >(
-            new CElementList(static_cast< CElement* >(
-            this->getDocumentElement().get()), tagname));
+        Reference< XNodeList > const xRet(
+            new CElementList(this->GetDocumentElement(), rTagname));
+        return xRet;
     }
 
     Reference< XNodeList > SAL_CALL CDocument::getElementsByTagNameNS(
-            const OUString& namespaceURI, const OUString& localName)
+            OUString const& rNamespaceURI, OUString const& rLocalName)
         throw (RuntimeException)
     {
-        return Reference< XNodeList >(
-            new CElementList(static_cast< CElement* >(
-            this->getDocumentElement().get()), namespaceURI, localName));
+        Reference< XNodeList > const xRet(
+            new CElementList(
+                this->GetDocumentElement(), rLocalName, &rNamespaceURI));
+        return xRet;
     }
 
     Reference< XDOMImplementation > SAL_CALL CDocument::getImplementation()
@@ -742,7 +752,7 @@ namespace DOM
         throw (RuntimeException, SAXException)
     {
         // add new namespaces to root node
-        xmlNodePtr pRoot = _getDocumentRootPtr(m_aDocPtr);
+        xmlNodePtr const pRoot = lcl_getDocumentRootPtr(m_aDocPtr);
         if (0 != pRoot) {
             const beans::StringPair * pSeq = i_rNamespaces.getConstArray();
             for (const beans::StringPair *pNsDef = pSeq;
@@ -769,7 +779,7 @@ namespace DOM
         throw (SAXException, RuntimeException)
     {
         // add new namespaces to root node
-        xmlNodePtr pRoot = _getDocumentRootPtr(m_aDocPtr);
+        xmlNodePtr const pRoot = lcl_getDocumentRootPtr(m_aDocPtr);
         if (0 != pRoot) {
             const beans::StringPair * pSeq = i_rNamespaces.getConstArray();
             for (const beans::StringPair *pNsDef = pSeq;

unoxml/source/dom/document.hxx

@@ -72,6 +72,8 @@ namespace DOM
         class CEventDispatcher;
     }
 
+    class CElement;
+
     typedef ::cppu::ImplInheritanceHelper6<
             CNode, XDocument, XDocumentEvent,
             XActiveDataControl, XActiveDataSource,
@@ -97,6 +99,7 @@ namespace DOM
         CDocument(xmlDocPtr aDocPtr);
 
         events::CEventDispatcher & GetEventDispatcher();
+        ::rtl::Reference< CElement > GetDocumentElement();
 
     public:
 

unoxml/source/dom/element.cxx

@@ -298,11 +298,12 @@ namespace DOM
     in the order in which they are
     encountered in a preorder traversal of this Element tree.
     */
-    Reference< XNodeList > CElement::getElementsByTagName(const OUString& name)
+    Reference< XNodeList > SAL_CALL
+    CElement::getElementsByTagName(OUString const& rLocalName)
         throw (RuntimeException)
     {
-        Reference< XNodeList > aList = Reference< XNodeList >(new CElementList(this, name));
-        return aList;
+        Reference< XNodeList > const xList(new CElementList(this, rLocalName));
+        return xList;
     }
 
     /**
@@ -310,12 +311,14 @@ namespace DOM
     name and namespace URI in the order in which they are encountered in
     a preorder traversal of this Element tree.
     */
-    Reference< XNodeList > CElement::getElementsByTagNameNS(const OUString& namespaceURI,
-            const OUString& localName)
+    Reference< XNodeList > SAL_CALL
+    CElement::getElementsByTagNameNS(
+            OUString const& rNamespaceURI, OUString const& rLocalName)
         throw (RuntimeException)
     {
-        Reference< XNodeList > aList = Reference< XNodeList >(new CElementList(this, localName, namespaceURI));
-        return aList;
+        Reference< XNodeList > const xList(
+            new CElementList(this, rLocalName, &rNamespaceURI));
+        return xList;
     }
 
     /**

unoxml/source/dom/elementlist.cxx

@@ -29,46 +29,40 @@
 
 #include <string.h>
 
+#include <element.hxx>
+
+
 namespace DOM
 {
 
-    CElementList::CElementList(const CElement* aElement, const OUString& aName)
-        : m_pElement(aElement)
-        , m_aName(aName)
-        , xURI(0)
-        , m_bRebuild(sal_True)
+    static xmlChar* lcl_initXmlString(::rtl::OUString const& rString)
     {
-        OString o1 = OUStringToOString(aName, RTL_TEXTENCODING_UTF8);
-        xName = new xmlChar[o1.getLength()];
-        strcpy((char*)xName, o1.getStr());
-        registerListener(aElement);
+        ::rtl::OString const os =
+            ::rtl::OUStringToOString(rString, RTL_TEXTENCODING_UTF8);
+        xmlChar *const pRet = new xmlChar[os.getLength() + 1];
+        strcpy(reinterpret_cast<char*>(pRet), os.getStr());
+        return pRet;
     }
 
-    CElementList::CElementList(const CElement* aElement, const OUString& aName, const OUString& aURI)
-        : m_pElement(aElement)
-        , m_aName(aName)
-        , m_aURI(aURI)
-        , m_bRebuild(sal_True)
+    CElementList::CElementList(::rtl::Reference<CElement> const& pElement,
+            OUString const& rName, OUString const*const pURI)
+        : m_pElement(pElement)
+        , m_pName(lcl_initXmlString(rName))
+        , m_pURI((pURI) ? lcl_initXmlString(*pURI) : 0)
+        , m_bRebuild(true)
     {
-        OString o1 = OUStringToOString(aName, RTL_TEXTENCODING_UTF8);
-        xName = new xmlChar[o1.getLength()];
-        strcpy((char*)xName, o1.getStr());
-        OString o2 = OUStringToOString(aURI, RTL_TEXTENCODING_UTF8);
-        xURI = new xmlChar[o2.getLength()];
-        strcpy((char*)xURI, o2.getStr());
-        registerListener(aElement);
+        registerListener(*m_pElement);
     }
 
-    void CElementList::registerListener(const CElement* pElement)
+    void CElementList::registerListener(CElement & rElement)
     {
         try {
-            // get the XNode
-            Reference< XNode > const xNode( CNode::getCNode(
-                    static_cast<const CNode*>(pElement)->m_aNodePtr).get() );
-            Reference< XEventTarget > xTarget(xNode, UNO_QUERY_THROW);
+            Reference< XEventTarget > const xTarget(
+                    static_cast<XElement*>(& rElement), UNO_QUERY_THROW);
             OUString aType = OUString::createFromAscii("DOMSubtreeModified");
             sal_Bool capture = sal_False;
-            xTarget->addEventListener(aType, Reference< XEventListener >(this), capture);
+            xTarget->addEventListener(aType,
+                    Reference< XEventListener >(this), capture);
         } catch (Exception &e){
             OString aMsg("Exception caught while registering NodeList as listener:\n");
             aMsg += OUStringToOString(e.Message, RTL_TEXTENCODING_ASCII_US);
@@ -85,20 +79,24 @@ namespace DOM
                 return;
             } else {
                 m_nodevector.erase(m_nodevector.begin(), m_nodevector.end());
-                m_bRebuild = sal_False; // don't rebuild until tree is mutated
+                m_bRebuild = false; // don't rebuild until tree is mutated
             }
         }
 
         while (pNode != NULL )
         {
             if (pNode->type == XML_ELEMENT_NODE &&
-                strcmp((char*)pNode->name, (char*)xName)==0)
+                (strcmp((char*)pNode->name, (char*)m_pName.get()) == 0))
             {
-                if (xURI == NULL)
+                if (!m_pURI) {
                     m_nodevector.push_back(pNode);
-                else
-                    if (pNode->ns != NULL && strcmp((char*)pNode->ns->href, (char*)xURI) == 0)
+                } else {
+                    if (pNode->ns != NULL && (0 ==
+                         strcmp((char*)pNode->ns->href, (char*)m_pURI.get())))
+                    {
                         m_nodevector.push_back(pNode);
+                    }
+                }
             }
             if (pNode->children != NULL) buildlist(pNode->children, sal_False);
 
@@ -113,25 +111,29 @@ namespace DOM
     sal_Int32 SAL_CALL CElementList::getLength() throw (RuntimeException)
     {
         // this has to be 'live'
-        buildlist(static_cast<const CNode*>(m_pElement)->m_aNodePtr);
+        buildlist(static_cast<const CNode*>(m_pElement.get())->m_aNodePtr);
         return m_nodevector.size();
     }
     /**
     Returns the indexth item in the collection.
     */
-    Reference< XNode > SAL_CALL CElementList::item(sal_Int32 index) throw (RuntimeException)
+    Reference< XNode > SAL_CALL CElementList::item(sal_Int32 index)
+        throw (RuntimeException)
     {
         if (index < 0) throw RuntimeException();
-        buildlist(static_cast<const CNode*>(m_pElement)->m_aNodePtr);
+        buildlist(static_cast<const CNode*>(m_pElement.get())->m_aNodePtr);
+        if (m_nodevector.size() <= static_cast<size_t>(index)) {
+            throw RuntimeException();
+        }
         Reference< XNode > const xRet(
             CNode::getCNode(m_nodevector[index]).get());
         return xRet;
     }
 
     // tree mutations can change the list
-    void SAL_CALL CElementList::handleEvent(const Reference< XEvent >& evt) throw (RuntimeException)
+    void SAL_CALL CElementList::handleEvent(Reference< XEvent > const&)
+        throw (RuntimeException)
     {
-      Reference< XEvent > aEvent = evt;
-        m_bRebuild = sal_True;
+        m_bRebuild = true;
     }
 }

unoxml/source/dom/elementlist.hxx

@@ -25,22 +25,27 @@
  *
  ************************************************************************/
 
-#ifndef _ELEMENTLIST_HXX
-#define _ELEMENTLIST_HXX
+#ifndef DOM_ELEMENTLIST_HXX
+#define DOM_ELEMENTLIST_HXX
 
 #include <vector>
+
+#include <boost/scoped_array.hpp>
+
+#include <libxml/tree.h>
+
 #include <sal/types.h>
-#include <cppuhelper/implbase1.hxx>
-#include <cppuhelper/implbase2.hxx>
+#include <rtl/ref.hxx>
+
 #include <com/sun/star/uno/Reference.h>
 #include <com/sun/star/uno/Exception.hpp>
 #include <com/sun/star/xml/dom/XNode.hpp>
 #include <com/sun/star/xml/dom/XNodeList.hpp>
 #include <com/sun/star/xml/dom/events/XEvent.hpp>
 #include <com/sun/star/xml/dom/events/XEventListener.hpp>
-#include "element.hxx"
-#include "document.hxx"
-#include "libxml/tree.h"
+
+#include <cppuhelper/implbase2.hxx>
+
 
 using ::rtl::OUString;
 using namespace com::sun::star::uno;
@@ -49,25 +54,28 @@ using namespace com::sun::star::xml::dom::events;
 
 namespace DOM
 {
-    typedef std::vector< xmlNodePtr > nodevector;
+    class CElement;
 
-    class CElementList : public cppu::WeakImplHelper2< XNodeList, com::sun::star::xml::dom::events::XEventListener >
+    typedef std::vector< xmlNodePtr > nodevector_t;
+
+    class CElementList
+        : public cppu::WeakImplHelper2< XNodeList,
+                com::sun::star::xml::dom::events::XEventListener >
     {
     private:
-        const CElement* m_pElement;
-        const OUString m_aName;
-        const OUString m_aURI;
-        xmlChar *xName;
-        xmlChar *xURI;
-        sal_Bool m_bRebuild;
-        nodevector m_nodevector;
+        ::rtl::Reference<CElement> const m_pElement;
+        ::boost::scoped_array<xmlChar> const m_pName;
+        ::boost::scoped_array<xmlChar> const m_pURI;
+        bool m_bRebuild;
+        nodevector_t m_nodevector;
 
         void buildlist(xmlNodePtr pNode, sal_Bool start=sal_True);
-        void registerListener(const CElement* pElement);
+        void registerListener(CElement & rElement);
 
     public:
-        CElementList(const CElement* aDoc, const OUString& aName);
-        CElementList(const CElement* aDoc, const OUString& aName, const OUString& aURI);
+        CElementList(::rtl::Reference<CElement> const& pElement,
+                OUString const& rName, OUString const*const pURI = 0);
+
         /**
         The number of nodes in the list.
         */
@@ -75,10 +83,12 @@ namespace DOM
         /**
         Returns the indexth item in the collection.
         */
-        virtual Reference< XNode > SAL_CALL item(sal_Int32 index) throw (RuntimeException);
+        virtual Reference< XNode > SAL_CALL item(sal_Int32 index)
+            throw (RuntimeException);
 
         // XEventListener
-        virtual void SAL_CALL handleEvent(const Reference< XEvent >& evt) throw (RuntimeException);
+        virtual void SAL_CALL handleEvent(const Reference< XEvent >& evt)
+            throw (RuntimeException);
     };
 }