From 6145d8c3f4a4da41b3fb2ad16627fd1f9d81fd5f Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:14 +0100 Subject: xmlfix3: #i113663#: unoxml: fix leaks of unlinked nodes --- unoxml/source/dom/node.cxx | 65 ++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 34 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index d4b317b425b3..35344b8fe72b 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -265,7 +265,8 @@ namespace DOM } CNode::CNode() - : m_aNodePtr(0) + : m_bUnlinked(false) + , m_aNodePtr(0) { } @@ -284,6 +285,10 @@ namespace DOM //remove from list if this wrapper goes away if (m_aNodePtr != 0) CNode::remove(m_aNodePtr); + // #i113663#: unlinked nodes will not be freed by xmlFreeDoc + if (m_bUnlinked) { + xmlFreeNode(m_aNodePtr); + } } static void _nsexchange(const xmlNodePtr aNode, xmlNsPtr oldNs, xmlNsPtr newNs) @@ -378,7 +383,7 @@ namespace DOM Reference< XNode > CNode::appendChild(const Reference< XNode >& newChild) throw (RuntimeException, DOMException) { - Reference< XNode> aNode; + CNode * pNode(0); if (m_aNodePtr != NULL) { xmlNodePtr cur = CNode::getNodePtr(newChild.get()); @@ -454,15 +459,16 @@ namespace DOM // because that will not remove unneeded ns decls _nscleanup(res, m_aNodePtr); - aNode = Reference< XNode>(CNode::get(res)); + pNode = CNode::get(res); } //XXX check for errors // dispatch DOMNodeInserted event, target is the new node // this node is the related node // does bubble - if (aNode.is()) + if (pNode) { + pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); Reference< XMutationEvent > event(docevent->createEvent( OUString::createFromAscii("DOMNodeInserted")), UNO_QUERY); @@ -474,7 +480,7 @@ namespace DOM // dispatch subtree modified for this node dispatchSubtreeModified(); } - return aNode; + return pNode; } /** @@ -484,15 +490,15 @@ namespace DOM Reference< XNode > CNode::cloneNode(sal_Bool bDeep) throw (RuntimeException) { - Reference< XNode> aNode; + CNode * pNode(0); if (m_aNodePtr != NULL) { - aNode = Reference< XNode>(CNode::get( - xmlCopyNode (m_aNodePtr, static_cast< int >(bDeep)) - )); + pNode = CNode::get( + xmlCopyNode(m_aNodePtr, (bDeep) ? 1 : 0)); + pNode->m_bUnlinked = true; // not linked yet } //XXX check for errors - return aNode; + return pNode; } /** @@ -766,6 +772,8 @@ namespace DOM cur->prev = pNewChild; if( pNewChild->prev != NULL) pNewChild->prev->next = pNewChild; + CNode *const pNode( CNode::get(pNewChild) ); + pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; } @@ -803,6 +811,9 @@ namespace DOM Reference< XNode > SAL_CALL CNode::removeChild(const Reference< XNode >& oldChild) throw (RuntimeException, DOMException) { + if (!oldChild.is()) { + throw RuntimeException(); + } if (oldChild->getParentNode() != Reference< XNode >(this)) { DOMException e; @@ -813,6 +824,8 @@ namespace DOM Reference xReturn( oldChild ); xmlNodePtr old = CNode::getNodePtr(oldChild); + CNode *const pOld( CNode::get(old) ); + pOld->m_bUnlinked = true; if( old->type == XML_ATTRIBUTE_NODE ) { @@ -822,30 +835,7 @@ namespace DOM } else { - - // update .last - if (m_aNodePtr->last == old) - m_aNodePtr->last = old->prev; - - xmlNodePtr cur = m_aNodePtr->children; - //find old node in child list - while (cur != NULL) - { - if(cur == old) - { - // unlink node from list - if (cur->prev != NULL) - cur->prev->next = cur->next; - if (cur->next != NULL) - cur->next->prev = cur->prev; - if (cur->parent != NULL && cur->parent->children == cur) - cur->parent->children = cur->next; - cur->prev = NULL; - cur->next = NULL; - cur->parent = NULL; - } - cur = cur->next; - } + xmlUnlinkNode(old); } /*DOMNodeRemoved @@ -880,6 +870,9 @@ namespace DOM const Reference< XNode >& newChild, const Reference< XNode >& oldChild) throw (RuntimeException, DOMException) { + if (!oldChild.is()) { + throw RuntimeException(); + } // XXX check node types if (oldChild->getParentNode() != Reference< XNode >(this)) { @@ -933,6 +926,10 @@ namespace DOM pOld->next = NULL; pOld->prev = NULL; pOld->parent = NULL; + CNode *const pOldNode( CNode::get(pOld) ); + pOldNode->m_bUnlinked = true; + CNode *const pNewNode( CNode::get(pNew) ); + pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; } -- cgit From 8f5bbf5b121339f21c3859b8476a994c4cfc841a Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:14 +0100 Subject: xmlfix3: #i113663#: unoxml: fix xmlRemoveProp errors --- unoxml/source/dom/node.cxx | 3 +++ 1 file changed, 3 insertions(+) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 35344b8fe72b..c14389ed4ba9 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -831,6 +831,7 @@ namespace DOM { xmlAttrPtr pAttr = (xmlAttrPtr) old; xmlRemoveProp( pAttr ); + pOld->m_aNodePtr = NULL; // freed by xmlRemoveProp xReturn.clear(); } else @@ -900,6 +901,8 @@ namespace DOM xmlAttrPtr pAttr = (xmlAttrPtr)pOld; xmlRemoveProp( pAttr ); + CNode *const pOldNode( CNode::get(pOld) ); + pOldNode->m_aNodePtr = NULL; // freed by xmlRemoveProp appendChild( newChild ); } else -- cgit From e2108d556163e7ca1b5bc8ebaa46d86f77f3fbee Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:15 +0100 Subject: xmlfix3: #i113663#: unoxml: fix leaks caused by CNode::get returning CNode*: CNode::getCNode now returns rtl::Reference, preventing leaks. --- unoxml/source/dom/node.cxx | 280 +++++++++++++++++++++++---------------------- 1 file changed, 144 insertions(+), 136 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index c14389ed4ba9..7f7c7eb11f67 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -158,99 +158,105 @@ namespace DOM } - CNode* CNode::get(const xmlNodePtr aNode, sal_Bool bCreate) + ::rtl::Reference + CNode::getCNode(xmlNodePtr const pNode, bool const bCreate) { - CNode* pNode = 0; - if (aNode == NULL) + if (0 == pNode) { return 0; + } //see CNode::remove ::osl::MutexGuard guard(NodeMutex::get()); //check whether there is already an instance for this node - nodemap_t::const_iterator i = CNode::theNodeMap.find(aNode); - if (i != CNode::theNodeMap.end()) - { - pNode = i->second; - } else - { + nodemap_t::const_iterator i = CNode::theNodeMap.find(pNode); + if (i != CNode::theNodeMap.end()) { + OSL_ASSERT(i->second); + return i->second; + } - // there is not yet an instance wrapping this node, - // create it and store it in the map - if (!bCreate) return NULL; + if (!bCreate) { return 0; } - switch (aNode->type) - { - case XML_ELEMENT_NODE: - // m_aNodeType = NodeType::ELEMENT_NODE; - pNode = static_cast< CNode* >(new CElement(aNode)); - break; - case XML_TEXT_NODE: - // m_aNodeType = NodeType::TEXT_NODE; - pNode = static_cast< CNode* >(new CText(aNode)); - break; - case XML_CDATA_SECTION_NODE: - // m_aNodeType = NodeType::CDATA_SECTION_NODE; - pNode = static_cast< CNode* >(new CCDATASection(aNode)); - break; - case XML_ENTITY_REF_NODE: - // m_aNodeType = NodeType::ENTITY_REFERENCE_NODE; - pNode = static_cast< CNode* >(new CEntityReference(aNode)); - break; - case XML_ENTITY_NODE: - // m_aNodeType = NodeType::ENTITY_NODE; - pNode = static_cast< CNode* >(new CEntity((xmlEntityPtr)aNode)); - break; - case XML_PI_NODE: - // m_aNodeType = NodeType::PROCESSING_INSTRUCTION_NODE; - pNode = static_cast< CNode* >(new CProcessingInstruction(aNode)); - break; - case XML_COMMENT_NODE: - // m_aNodeType = NodeType::COMMENT_NODE; - pNode = static_cast< CNode* >(new CComment(aNode)); - break; - case XML_DOCUMENT_NODE: - // m_aNodeType = NodeType::DOCUMENT_NODE; - pNode = static_cast< CNode* >(new CDocument((xmlDocPtr)aNode)); - break; - case XML_DOCUMENT_TYPE_NODE: - case XML_DTD_NODE: - // m_aNodeType = NodeType::DOCUMENT_TYPE_NODE; - pNode = static_cast< CNode* >(new CDocumentType((xmlDtdPtr)aNode)); - break; - case XML_DOCUMENT_FRAG_NODE: - // m_aNodeType = NodeType::DOCUMENT_FRAGMENT_NODE; - pNode = static_cast< CNode* >(new CDocumentFragment(aNode)); - break; - case XML_NOTATION_NODE: - // m_aNodeType = NodeType::NOTATION_NODE; - pNode = static_cast< CNode* >(new CNotation((xmlNotationPtr)aNode)); - break; - case XML_ATTRIBUTE_NODE: - // m_aNodeType = NodeType::NOTATION_NODE; - pNode = static_cast< CNode* >(new CAttr((xmlAttrPtr)aNode)); - break; - // unsupported node types - case XML_HTML_DOCUMENT_NODE: - case XML_ELEMENT_DECL: - case XML_ATTRIBUTE_DECL: - case XML_ENTITY_DECL: - case XML_NAMESPACE_DECL: - default: - pNode = 0; - break; - } + // there is not yet an instance wrapping this node, + // create it and store it in the map - if ( pNode != 0 ) - { - if(!CNode::theNodeMap.insert(nodemap_t::value_type(aNode, pNode)).second) - { - // if insertion failed, delete the new instance and return null - delete pNode; - pNode = 0; - } + ::rtl::Reference pCNode; + switch (pNode->type) + { + case XML_ELEMENT_NODE: + // m_aNodeType = NodeType::ELEMENT_NODE; + pCNode = static_cast< CNode* >(new CElement(pNode)); + break; + case XML_TEXT_NODE: + // m_aNodeType = NodeType::TEXT_NODE; + pCNode = static_cast< CNode* >(new CText(pNode)); + break; + case XML_CDATA_SECTION_NODE: + // m_aNodeType = NodeType::CDATA_SECTION_NODE; + pCNode = static_cast< CNode* >(new CCDATASection(pNode)); + break; + case XML_ENTITY_REF_NODE: + // m_aNodeType = NodeType::ENTITY_REFERENCE_NODE; + pCNode = static_cast< CNode* >(new CEntityReference(pNode)); + break; + case XML_ENTITY_NODE: + // m_aNodeType = NodeType::ENTITY_NODE; + pCNode = static_cast< CNode* >(new CEntity( + reinterpret_cast(pNode))); + break; + case XML_PI_NODE: + // m_aNodeType = NodeType::PROCESSING_INSTRUCTION_NODE; + pCNode = static_cast< CNode* >(new CProcessingInstruction(pNode)); + break; + case XML_COMMENT_NODE: + // m_aNodeType = NodeType::COMMENT_NODE; + pCNode = static_cast< CNode* >(new CComment(pNode)); + break; + case XML_DOCUMENT_NODE: + // m_aNodeType = NodeType::DOCUMENT_NODE; + pCNode = static_cast< CNode* >(new CDocument( + reinterpret_cast(pNode))); + break; + case XML_DOCUMENT_TYPE_NODE: + case XML_DTD_NODE: + // m_aNodeType = NodeType::DOCUMENT_TYPE_NODE; + pCNode = static_cast< CNode* >(new CDocumentType( + reinterpret_cast(pNode))); + break; + case XML_DOCUMENT_FRAG_NODE: + // m_aNodeType = NodeType::DOCUMENT_FRAGMENT_NODE; + pCNode = static_cast< CNode* >(new CDocumentFragment(pNode)); + break; + case XML_NOTATION_NODE: + // m_aNodeType = NodeType::NOTATION_NODE; + pCNode = static_cast< CNode* >(new CNotation( + reinterpret_cast(pNode))); + break; + case XML_ATTRIBUTE_NODE: + // m_aNodeType = NodeType::ATTRIBUTE_NODE; + pCNode = static_cast< CNode* >(new CAttr( + reinterpret_cast(pNode))); + break; + // unsupported node types + case XML_HTML_DOCUMENT_NODE: + case XML_ELEMENT_DECL: + case XML_ATTRIBUTE_DECL: + case XML_ENTITY_DECL: + case XML_NAMESPACE_DECL: + default: + break; + } + + if (pCNode != 0) { + bool const bInserted = CNode::theNodeMap.insert( + nodemap_t::value_type(pNode, pCNode.get())).second; + OSL_ASSERT(bInserted); + if (!bInserted) { + // if insertion failed, delete new instance and return null + return 0; } } - OSL_ENSURE(pNode, "no node produced during CNode::get!"); - return pNode; + + OSL_ENSURE(pCNode.is(), "no node produced during CNode::getCNode!"); + return pCNode; } xmlNodePtr CNode::getNodePtr(const Reference< XNode >& aNode) @@ -383,7 +389,7 @@ namespace DOM Reference< XNode > CNode::appendChild(const Reference< XNode >& newChild) throw (RuntimeException, DOMException) { - CNode * pNode(0); + ::rtl::Reference pNode; if (m_aNodePtr != NULL) { xmlNodePtr cur = CNode::getNodePtr(newChild.get()); @@ -459,28 +465,29 @@ namespace DOM // because that will not remove unneeded ns decls _nscleanup(res, m_aNodePtr); - pNode = CNode::get(res); + pNode = CNode::getCNode(res); } //XXX check for errors // dispatch DOMNodeInserted event, target is the new node // this node is the related node // does bubble - if (pNode) + if (pNode.is()) { pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); Reference< XMutationEvent > event(docevent->createEvent( OUString::createFromAscii("DOMNodeInserted")), UNO_QUERY); event->initMutationEvent(OUString::createFromAscii("DOMNodeInserted") - , sal_True, sal_False, Reference< XNode >(CNode::get(m_aNodePtr)), + , sal_True, sal_False, + Reference< XNode >(CNode::getCNode(m_aNodePtr).get()), OUString(), OUString(), OUString(), (AttrChangeType)0 ); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); // dispatch subtree modified for this node dispatchSubtreeModified(); } - return pNode; + return pNode.get(); } /** @@ -490,15 +497,14 @@ namespace DOM Reference< XNode > CNode::cloneNode(sal_Bool bDeep) throw (RuntimeException) { - CNode * pNode(0); - if (m_aNodePtr != NULL) - { - pNode = CNode::get( - xmlCopyNode(m_aNodePtr, (bDeep) ? 1 : 0)); - pNode->m_bUnlinked = true; // not linked yet + if (0 == m_aNodePtr) { + return 0; } + ::rtl::Reference const pNode = CNode::getCNode( + xmlCopyNode(m_aNodePtr, (bDeep) ? 1 : 0)); + pNode->m_bUnlinked = true; // not linked yet //XXX check for errors - return pNode; + return pNode.get(); } /** @@ -525,13 +531,13 @@ namespace DOM Reference< XNodeList > CNode::getChildNodes() throw (RuntimeException) { - Reference< XNodeList > aNodeList; - if (m_aNodePtr != NULL) - { - aNodeList = Reference< XNodeList >(new CChildList(CNode::get(m_aNodePtr))); + if (0 == m_aNodePtr) { + return 0; } + Reference< XNodeList > const xNodeList( + new CChildList(*CNode::getCNode(m_aNodePtr))); // XXX check for errors? - return aNodeList; + return xNodeList; } /** @@ -540,11 +546,12 @@ namespace DOM Reference< XNode > CNode::getFirstChild() throw (RuntimeException) { - Reference< XNode > aNode; - if (m_aNodePtr != NULL) { - aNode = Reference< XNode >(CNode::get(m_aNodePtr->children)); + if (0 == m_aNodePtr) { + return 0; } - return aNode; + Reference< XNode > const xNode( + CNode::getCNode(m_aNodePtr->children).get()); + return xNode; } /** @@ -553,11 +560,12 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getLastChild() throw (RuntimeException) { - Reference< XNode > aNode; - if (m_aNodePtr != NULL) { - aNode = Reference< XNode >(CNode::get(xmlGetLastChild(m_aNodePtr))); + if (0 == m_aNodePtr) { + return 0; } - return aNode; + Reference< XNode > const xNode( + CNode::getCNode(xmlGetLastChild(m_aNodePtr)).get()); + return xNode; } /** @@ -603,12 +611,11 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getNextSibling() throw (RuntimeException) { - Reference< XNode > aNode; - if(m_aNodePtr != NULL) - { - aNode = Reference< XNode >(CNode::get(m_aNodePtr->next)); + if (0 == m_aNodePtr) { + return 0; } - return aNode; + Reference< XNode > const xNode(CNode::getCNode(m_aNodePtr->next).get()); + return xNode; } /** @@ -664,14 +671,14 @@ namespace DOM Reference< XDocument > SAL_CALL CNode::getOwnerDocument() throw (RuntimeException) { - Reference aDoc; - if (m_aNodePtr != NULL) - { - aDoc = Reference< XDocument >(static_cast< CDocument* >( - CNode::get((xmlNodePtr)m_aNodePtr->doc))); + if (0 == m_aNodePtr) { + return 0; } - return aDoc; - + Reference< XDocument > const xDoc( + static_cast(CNode::getCNode( + reinterpret_cast(m_aNodePtr->doc)).get()), + UNO_QUERY_THROW); + return xDoc; } /** @@ -680,12 +687,12 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getParentNode() throw (RuntimeException) { - Reference aNode; - if (m_aNodePtr != NULL) - { - aNode = Reference< XNode >(CNode::get(m_aNodePtr->parent)); + if (0 == m_aNodePtr) { + return 0; } - return aNode; + Reference< XNode > const xNode( + CNode::getCNode(m_aNodePtr->parent).get()); + return xNode; } /** @@ -713,12 +720,12 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getPreviousSibling() throw (RuntimeException) { - Reference< XNode > aNode; - if (m_aNodePtr != NULL) - { - aNode = Reference< XNode >(CNode::get(m_aNodePtr->prev)); + if (0 == m_aNodePtr) { + return 0; } - return aNode; + Reference< XNode > const xNode( + CNode::getCNode(m_aNodePtr->prev).get()); + return xNode; } /** @@ -772,7 +779,7 @@ namespace DOM cur->prev = pNewChild; if( pNewChild->prev != NULL) pNewChild->prev->next = pNewChild; - CNode *const pNode( CNode::get(pNewChild) ); + ::rtl::Reference const pNode(CNode::getCNode(pNewChild)); pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; @@ -824,7 +831,7 @@ namespace DOM Reference xReturn( oldChild ); xmlNodePtr old = CNode::getNodePtr(oldChild); - CNode *const pOld( CNode::get(old) ); + ::rtl::Reference const pOld( CNode::getCNode(old) ); pOld->m_bUnlinked = true; if( old->type == XML_ATTRIBUTE_NODE ) @@ -853,7 +860,8 @@ namespace DOM Reference< XMutationEvent > event(docevent->createEvent( OUString::createFromAscii("DOMNodeRemoved")), UNO_QUERY); event->initMutationEvent(OUString::createFromAscii("DOMNodeRemoved"), sal_True, - sal_False, Reference< XNode >(CNode::get(m_aNodePtr)), + sal_False, + Reference< XNode >(CNode::getCNode(m_aNodePtr).get()), OUString(), OUString(), OUString(), (AttrChangeType)0 ); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); @@ -901,7 +909,7 @@ namespace DOM xmlAttrPtr pAttr = (xmlAttrPtr)pOld; xmlRemoveProp( pAttr ); - CNode *const pOldNode( CNode::get(pOld) ); + ::rtl::Reference const pOldNode( CNode::getCNode(pOld) ); pOldNode->m_aNodePtr = NULL; // freed by xmlRemoveProp appendChild( newChild ); } @@ -929,9 +937,9 @@ namespace DOM pOld->next = NULL; pOld->prev = NULL; pOld->parent = NULL; - CNode *const pOldNode( CNode::get(pOld) ); + ::rtl::Reference const pOldNode(CNode::getCNode(pOld)); pOldNode->m_bUnlinked = true; - CNode *const pNewNode( CNode::get(pNew) ); + ::rtl::Reference const pNewNode(CNode::getCNode(pNew)); pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; -- cgit From c33c26cbd26310ed03aadfb6fa361f10febb56d5 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:16 +0100 Subject: xmlfix3: #i113681#: unoxml: fix races in global node map: put a WeakReference into nodemap_t, and check it in getCNode. check for race between removeCNode and getCNode in removeCNode. remove pointless call of removeCNode. --- unoxml/source/dom/node.cxx | 74 +++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 30 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 7f7c7eb11f67..58c6a96b51db 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -132,32 +132,30 @@ namespace DOM return nNamespaceToken; } + typedef std::map< const xmlNodePtr, + ::std::pair< WeakReference, CNode* > > nodemap_t; + static nodemap_t g_theNodeMap; - nodemap_t CNode::theNodeMap; - - void CNode::remove(const xmlNodePtr aNode) + void CNode::removeCNode(xmlNodePtr const pNode, CNode const*const pCNode) { - //Using the guard here protects against races when at the same time - //CNode::get() is called. This fix helps in many cases but is still - //incorrect. remove is called from ~CNode. That is, while the object - //is being destructed it can still be obtained by calling CNode::get(). - //Another bug currently prevents the correct destruction of CNodes. So - //the destructor is rarely called. - // - //Doing this right would probably mean to store WeakReferences in the - //map and also guard oder functions. To keep the risk at a minimum - //we keep this imperfect fix for the upcoming release and fix it later - //properly (http://qa.openoffice.org/issues/show_bug.cgi?id=113682) ::osl::MutexGuard guard(NodeMutex::get()); - nodemap_t::iterator i = CNode::theNodeMap.find(aNode); - if (i != CNode::theNodeMap.end()) - { - // CNode *pNode = i->second; - CNode::theNodeMap.erase(i); + nodemap_t::iterator const i = g_theNodeMap.find(pNode); + if (i != g_theNodeMap.end()) { + // #i113681# consider this scenario: + // T1 calls ~CNode + // T2 calls getCNode: lookup will find i->second->first invalid + // so a new CNode is created and inserted + // T1 calls removeCNode: i->second->second now points to a + // different CNode instance! + // + // check that the CNode is the right one + CNode *const pCurrent = i->second.second; + if (pCurrent == pCNode) { + g_theNodeMap.erase(i); + } } } - ::rtl::Reference CNode::getCNode(xmlNodePtr const pNode, bool const bCreate) { @@ -167,10 +165,16 @@ namespace DOM //see CNode::remove ::osl::MutexGuard guard(NodeMutex::get()); //check whether there is already an instance for this node - nodemap_t::const_iterator i = CNode::theNodeMap.find(pNode); - if (i != CNode::theNodeMap.end()) { - OSL_ASSERT(i->second); - return i->second; + nodemap_t::const_iterator const i = g_theNodeMap.find(pNode); + if (i != g_theNodeMap.end()) { + // #i113681# check that the CNode is still alive + uno::Reference const xNode(i->second.first); + if (xNode.is()) + { + ::rtl::Reference ret(i->second.second); + OSL_ASSERT(ret.is()); + return ret; + } } if (!bCreate) { return 0; } @@ -246,8 +250,11 @@ namespace DOM } if (pCNode != 0) { - bool const bInserted = CNode::theNodeMap.insert( - nodemap_t::value_type(pNode, pCNode.get())).second; + bool const bInserted = g_theNodeMap.insert( + nodemap_t::value_type(pNode, + ::std::make_pair(WeakReference(pCNode.get()), + pCNode.get())) + ).second; OSL_ASSERT(bInserted); if (!bInserted) { // if insertion failed, delete new instance and return null @@ -289,8 +296,9 @@ namespace DOM CNode::~CNode() { //remove from list if this wrapper goes away - if (m_aNodePtr != 0) - CNode::remove(m_aNodePtr); + if (m_aNodePtr != 0) { + CNode::removeCNode(m_aNodePtr, this); + } // #i113663#: unlinked nodes will not be freed by xmlFreeDoc if (m_bUnlinked) { xmlFreeNode(m_aNodePtr); @@ -457,8 +465,14 @@ namespace DOM // libxml can do optimizations, when appending nodes. // if res != cur, something was optimized and the newchild-wrapper // should be updated - if (cur != res) - CNode::remove(cur); + if (res && (cur != res)) { + ::rtl::Reference pCNode(CNode::getCNode(cur)); + OSL_ASSERT(pCNode.is()); + if (pCNode.is()) { + pCNode->m_aNodePtr = 0; // has been freed + CNode::removeCNode(cur, pCNode.get()); + } + } // use custom ns cleanup instaead of // xmlReconciliateNs(m_aNodePtr->doc, m_aNodePtr); -- cgit From cb5905f108fb7bb99587791fca24c9d481ae93d9 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:16 +0100 Subject: xmlfix3: unoxml: new method CNode::invalidate --- unoxml/source/dom/node.cxx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 58c6a96b51db..f122b62dd9e2 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -293,7 +293,7 @@ namespace DOM m_rDocument = getOwnerDocument(); } - CNode::~CNode() + void CNode::invalidate() { //remove from list if this wrapper goes away if (m_aNodePtr != 0) { @@ -303,6 +303,12 @@ namespace DOM if (m_bUnlinked) { xmlFreeNode(m_aNodePtr); } + m_aNodePtr = 0; + } + + CNode::~CNode() + { + invalidate(); } static void _nsexchange(const xmlNodePtr aNode, xmlNsPtr oldNs, xmlNsPtr newNs) @@ -469,8 +475,7 @@ namespace DOM ::rtl::Reference pCNode(CNode::getCNode(cur)); OSL_ASSERT(pCNode.is()); if (pCNode.is()) { - pCNode->m_aNodePtr = 0; // has been freed - CNode::removeCNode(cur, pCNode.get()); + pCNode->invalidate(); // cur has been freed } } @@ -852,7 +857,7 @@ namespace DOM { xmlAttrPtr pAttr = (xmlAttrPtr) old; xmlRemoveProp( pAttr ); - pOld->m_aNodePtr = NULL; // freed by xmlRemoveProp + pOld->invalidate(); // freed by xmlRemoveProp xReturn.clear(); } else @@ -924,7 +929,7 @@ namespace DOM xmlAttrPtr pAttr = (xmlAttrPtr)pOld; xmlRemoveProp( pAttr ); ::rtl::Reference const pOldNode( CNode::getCNode(pOld) ); - pOldNode->m_aNodePtr = NULL; // freed by xmlRemoveProp + pOldNode->invalidate(); // freed by xmlRemoveProp appendChild( newChild ); } else -- cgit From a10e0ee65f2721aadae0d1e6cc26f749e3ac6bf6 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:19 +0100 Subject: xmlfix3: unoxml: refactor CNode initialization --- unoxml/source/dom/node.cxx | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index f122b62dd9e2..776158bab190 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -277,20 +277,17 @@ namespace DOM return 0; } - CNode::CNode() - : m_bUnlinked(false) - , m_aNodePtr(0) - { - } - - void CNode::init_node(const xmlNodePtr aNode) - { - m_aNodePtr = aNode; + CNode::CNode(NodeType const& reNodeType, xmlNodePtr const& rpNode) + : m_bUnlinked(false) + , m_aNodeType(reNodeType) + , m_aNodePtr(rpNode) // keep containing document alive - // (if we are not that document ourselves) - if (m_aNodePtr->type != XML_DOCUMENT_NODE) - m_rDocument = getOwnerDocument(); + // (but not if this is a document; that would create a leak!) + , m_xDocument( (m_aNodePtr->type != XML_DOCUMENT_NODE) + ? getOwnerDocument() : 0 ) + { + OSL_ASSERT(m_aNodePtr); } void CNode::invalidate() -- cgit From 8ab417f83b7a6ee8a11deb5f534d6fc6c6fd9899 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:23 +0100 Subject: xmlfix3: #i113682#: unoxml: no more globals in CEventDispatcher: instead CDocument now has a CEventDispatcher member. --- unoxml/source/dom/node.cxx | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 776158bab190..3554811d8b45 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -285,7 +285,7 @@ namespace DOM // keep containing document alive // (but not if this is a document; that would create a leak!) , m_xDocument( (m_aNodePtr->type != XML_DOCUMENT_NODE) - ? getOwnerDocument() : 0 ) + ? GetOwnerDocument_Impl() : 0 ) { OSL_ASSERT(m_aNodePtr); } @@ -308,6 +308,18 @@ namespace DOM invalidate(); } + ::rtl::Reference< CDocument > CNode::GetOwnerDocument_Impl() + { + if (0 == m_aNodePtr) { + return 0; + } + ::rtl::Reference< CDocument > const xDoc( + dynamic_cast(CNode::getCNode( + reinterpret_cast(m_aNodePtr->doc)).get())); + return xDoc; + } + + static void _nsexchange(const xmlNodePtr aNode, xmlNsPtr oldNs, xmlNsPtr newNs) { // recursively exchange any references to oldNs with references to newNs @@ -687,13 +699,7 @@ namespace DOM Reference< XDocument > SAL_CALL CNode::getOwnerDocument() throw (RuntimeException) { - if (0 == m_aNodePtr) { - return 0; - } - Reference< XDocument > const xDoc( - static_cast(CNode::getCNode( - reinterpret_cast(m_aNodePtr->doc)).get()), - UNO_QUERY_THROW); + Reference< XDocument > const xDoc(GetOwnerDocument_Impl().get()); return xDoc; } @@ -1016,7 +1022,9 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { - events::CEventDispatcher::addListener(m_aNodePtr, eventType, listener, useCapture); + events::CEventDispatcher & rDispatcher( + m_xDocument->GetEventDispatcher()); + rDispatcher.addListener(m_aNodePtr, eventType, listener, useCapture); } void SAL_CALL CNode::removeEventListener(const OUString& eventType, @@ -1024,13 +1032,17 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { - events::CEventDispatcher::removeListener(m_aNodePtr, eventType, listener, useCapture); + events::CEventDispatcher & rDispatcher( + m_xDocument->GetEventDispatcher()); + rDispatcher.removeListener(m_aNodePtr, eventType, listener, useCapture); } sal_Bool SAL_CALL CNode::dispatchEvent(const Reference< XEvent >& evt) throw(RuntimeException, EventException) { - events::CEventDispatcher::dispatchEvent(m_aNodePtr, evt); + events::CEventDispatcher & rDispatcher( + m_xDocument->GetEventDispatcher()); + rDispatcher.dispatchEvent(this, evt); return sal_True; } -- cgit From aa85497de77acae2c9e58d7339584dcd7665e425 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:24 +0100 Subject: xmlfix3: unoxml: fix CChildList: member pointer does not keep document alive --- unoxml/source/dom/node.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 3554811d8b45..2b2885778ff4 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -562,9 +562,7 @@ namespace DOM if (0 == m_aNodePtr) { return 0; } - Reference< XNodeList > const xNodeList( - new CChildList(*CNode::getCNode(m_aNodePtr))); - // XXX check for errors? + Reference< XNodeList > const xNodeList(new CChildList(this)); return xNodeList; } -- cgit From 06ebc79a5d723211d55be7a1c014299e71acfc20 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:25 +0100 Subject: xmlfix3: unoxml: replace CNode XUnoTunnel implementation with something sane --- unoxml/source/dom/node.cxx | 59 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 9 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 2b2885778ff4..0aa42d5d6d17 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -25,9 +25,21 @@ * ************************************************************************/ +#include + #include #include -#include "node.hxx" + +#include + +#include + +#include +#include +#include + +#include + #include "element.hxx" #include "text.hxx" #include "cdatasection.hxx" @@ -42,18 +54,28 @@ #include "childlist.hxx" #include "attr.hxx" -#include -#include "rtl/instance.hxx" -#include "osl/mutex.hxx" #include "../events/eventdispatcher.hxx" #include "../events/mutationevent.hxx" -#include -#include + + +using namespace ::com::sun::star; + namespace { //see CNode::remove struct NodeMutex: public ::rtl::Static {}; + struct UnoTunnelId + : public ::rtl::StaticWithInit< Sequence, UnoTunnelId > + { + Sequence operator() () + { + Sequence ret(16); + rtl_createUuid( + reinterpret_cast(ret.getArray()), 0, sal_True); + return ret; + } + }; } namespace DOM @@ -269,7 +291,7 @@ namespace DOM xmlNodePtr CNode::getNodePtr(const Reference< XNode >& aNode) { try { - CNode* pNode=dynamic_cast(aNode.get()); + CNode *const pNode = GetImplementation(aNode); if( pNode ) return pNode->m_aNodePtr; } @@ -308,6 +330,17 @@ namespace DOM invalidate(); } + CNode * + CNode::GetImplementation(uno::Reference const& xNode) + { + uno::Reference const xUnoTunnel(xNode, UNO_QUERY); + if (!xUnoTunnel.is()) { return 0; } + CNode *const pCNode( reinterpret_cast< CNode* >( + ::sal::static_int_cast< sal_IntPtr >( + xUnoTunnel->getSomething(UnoTunnelId::get())))); + return pCNode; + } + ::rtl::Reference< CDocument > CNode::GetOwnerDocument_Impl() { if (0 == m_aNodePtr) { @@ -1044,10 +1077,18 @@ namespace DOM return sal_True; } - ::sal_Int64 SAL_CALL CNode::getSomething(const Sequence< ::sal_Int8 >& /*aIdentifier*/) + ::sal_Int64 SAL_CALL + CNode::getSomething(Sequence< ::sal_Int8 > const& rId) throw (RuntimeException) { - return sal::static_int_cast(reinterpret_cast(m_aNodePtr)); + if ((rId.getLength() == 16) && + (0 == rtl_compareMemory(UnoTunnelId::get().getConstArray(), + rId.getConstArray(), 16))) + { + return ::sal::static_int_cast< sal_Int64 >( + reinterpret_cast< sal_IntPtr >(this) ); + } + return 0; } } -- cgit From 46716bcf7fd75a2a293722a6a458e4138c91f394 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:25 +0100 Subject: xmlfix3: unoxml: all node ctors get CDocument parameter (and in the next patch, an actual CDocument, not just 0 :) --- unoxml/source/dom/node.cxx | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 0aa42d5d6d17..85de84c71e30 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -209,32 +209,32 @@ namespace DOM { case XML_ELEMENT_NODE: // m_aNodeType = NodeType::ELEMENT_NODE; - pCNode = static_cast< CNode* >(new CElement(pNode)); + pCNode = static_cast< CNode* >(new CElement(*(CDocument*)0, pNode)); break; case XML_TEXT_NODE: // m_aNodeType = NodeType::TEXT_NODE; - pCNode = static_cast< CNode* >(new CText(pNode)); + pCNode = static_cast< CNode* >(new CText(*(CDocument*)0, pNode)); break; case XML_CDATA_SECTION_NODE: // m_aNodeType = NodeType::CDATA_SECTION_NODE; - pCNode = static_cast< CNode* >(new CCDATASection(pNode)); + pCNode = static_cast< CNode* >(new CCDATASection(*(CDocument*)0, pNode)); break; case XML_ENTITY_REF_NODE: // m_aNodeType = NodeType::ENTITY_REFERENCE_NODE; - pCNode = static_cast< CNode* >(new CEntityReference(pNode)); + pCNode = static_cast< CNode* >(new CEntityReference(*(CDocument*)0, pNode)); break; case XML_ENTITY_NODE: // m_aNodeType = NodeType::ENTITY_NODE; - pCNode = static_cast< CNode* >(new CEntity( + pCNode = static_cast< CNode* >(new CEntity(*(CDocument*)0, reinterpret_cast(pNode))); break; case XML_PI_NODE: // m_aNodeType = NodeType::PROCESSING_INSTRUCTION_NODE; - pCNode = static_cast< CNode* >(new CProcessingInstruction(pNode)); + pCNode = static_cast< CNode* >(new CProcessingInstruction(*(CDocument*)0, pNode)); break; case XML_COMMENT_NODE: // m_aNodeType = NodeType::COMMENT_NODE; - pCNode = static_cast< CNode* >(new CComment(pNode)); + pCNode = static_cast< CNode* >(new CComment(*(CDocument*)0, pNode)); break; case XML_DOCUMENT_NODE: // m_aNodeType = NodeType::DOCUMENT_NODE; @@ -244,21 +244,21 @@ namespace DOM case XML_DOCUMENT_TYPE_NODE: case XML_DTD_NODE: // m_aNodeType = NodeType::DOCUMENT_TYPE_NODE; - pCNode = static_cast< CNode* >(new CDocumentType( + pCNode = static_cast< CNode* >(new CDocumentType(*(CDocument*)0, reinterpret_cast(pNode))); break; case XML_DOCUMENT_FRAG_NODE: // m_aNodeType = NodeType::DOCUMENT_FRAGMENT_NODE; - pCNode = static_cast< CNode* >(new CDocumentFragment(pNode)); + pCNode = static_cast< CNode* >(new CDocumentFragment(*(CDocument*)0, pNode)); break; case XML_NOTATION_NODE: // m_aNodeType = NodeType::NOTATION_NODE; - pCNode = static_cast< CNode* >(new CNotation( + pCNode = static_cast< CNode* >(new CNotation(*(CDocument*)0, reinterpret_cast(pNode))); break; case XML_ATTRIBUTE_NODE: // m_aNodeType = NodeType::ATTRIBUTE_NODE; - pCNode = static_cast< CNode* >(new CAttr( + pCNode = static_cast< CNode* >(new CAttr(*(CDocument*)0, reinterpret_cast(pNode))); break; // unsupported node types @@ -300,14 +300,15 @@ namespace DOM } - CNode::CNode(NodeType const& reNodeType, xmlNodePtr const& rpNode) + CNode::CNode(CDocument const& rDocument, + NodeType const& reNodeType, xmlNodePtr const& rpNode) : m_bUnlinked(false) , m_aNodeType(reNodeType) , m_aNodePtr(rpNode) // keep containing document alive // (but not if this is a document; that would create a leak!) , m_xDocument( (m_aNodePtr->type != XML_DOCUMENT_NODE) - ? GetOwnerDocument_Impl() : 0 ) + ? &const_cast(rDocument) : 0 ) { OSL_ASSERT(m_aNodePtr); } -- cgit From dd377d72a5ef66daee850b66ab7b7308939a7626 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:25 +0100 Subject: xmlfix3: #i113682#: unoxml: no more globals in CNode: instead now the CDocument contains a node map member. --- unoxml/source/dom/node.cxx | 264 ++++++++++----------------------------------- 1 file changed, 54 insertions(+), 210 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 85de84c71e30..f16babb3ab2f 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -40,19 +40,8 @@ #include -#include "element.hxx" -#include "text.hxx" -#include "cdatasection.hxx" -#include "entityreference.hxx" -#include "entity.hxx" -#include "processinginstruction.hxx" -#include "comment.hxx" -#include "document.hxx" -#include "documenttype.hxx" -#include "documentfragment.hxx" -#include "notation.hxx" -#include "childlist.hxx" -#include "attr.hxx" +#include +#include #include "../events/eventdispatcher.hxx" #include "../events/mutationevent.hxx" @@ -63,8 +52,6 @@ using namespace ::com::sun::star; namespace { -//see CNode::remove - struct NodeMutex: public ::rtl::Static {}; struct UnoTunnelId : public ::rtl::StaticWithInit< Sequence, UnoTunnelId > { @@ -154,151 +141,6 @@ namespace DOM return nNamespaceToken; } - typedef std::map< const xmlNodePtr, - ::std::pair< WeakReference, CNode* > > nodemap_t; - static nodemap_t g_theNodeMap; - - void CNode::removeCNode(xmlNodePtr const pNode, CNode const*const pCNode) - { - ::osl::MutexGuard guard(NodeMutex::get()); - nodemap_t::iterator const i = g_theNodeMap.find(pNode); - if (i != g_theNodeMap.end()) { - // #i113681# consider this scenario: - // T1 calls ~CNode - // T2 calls getCNode: lookup will find i->second->first invalid - // so a new CNode is created and inserted - // T1 calls removeCNode: i->second->second now points to a - // different CNode instance! - // - // check that the CNode is the right one - CNode *const pCurrent = i->second.second; - if (pCurrent == pCNode) { - g_theNodeMap.erase(i); - } - } - } - - ::rtl::Reference - CNode::getCNode(xmlNodePtr const pNode, bool const bCreate) - { - if (0 == pNode) { - return 0; - } - //see CNode::remove - ::osl::MutexGuard guard(NodeMutex::get()); - //check whether there is already an instance for this node - nodemap_t::const_iterator const i = g_theNodeMap.find(pNode); - if (i != g_theNodeMap.end()) { - // #i113681# check that the CNode is still alive - uno::Reference const xNode(i->second.first); - if (xNode.is()) - { - ::rtl::Reference ret(i->second.second); - OSL_ASSERT(ret.is()); - return ret; - } - } - - if (!bCreate) { return 0; } - - // there is not yet an instance wrapping this node, - // create it and store it in the map - - ::rtl::Reference pCNode; - switch (pNode->type) - { - case XML_ELEMENT_NODE: - // m_aNodeType = NodeType::ELEMENT_NODE; - pCNode = static_cast< CNode* >(new CElement(*(CDocument*)0, pNode)); - break; - case XML_TEXT_NODE: - // m_aNodeType = NodeType::TEXT_NODE; - pCNode = static_cast< CNode* >(new CText(*(CDocument*)0, pNode)); - break; - case XML_CDATA_SECTION_NODE: - // m_aNodeType = NodeType::CDATA_SECTION_NODE; - pCNode = static_cast< CNode* >(new CCDATASection(*(CDocument*)0, pNode)); - break; - case XML_ENTITY_REF_NODE: - // m_aNodeType = NodeType::ENTITY_REFERENCE_NODE; - pCNode = static_cast< CNode* >(new CEntityReference(*(CDocument*)0, pNode)); - break; - case XML_ENTITY_NODE: - // m_aNodeType = NodeType::ENTITY_NODE; - pCNode = static_cast< CNode* >(new CEntity(*(CDocument*)0, - reinterpret_cast(pNode))); - break; - case XML_PI_NODE: - // m_aNodeType = NodeType::PROCESSING_INSTRUCTION_NODE; - pCNode = static_cast< CNode* >(new CProcessingInstruction(*(CDocument*)0, pNode)); - break; - case XML_COMMENT_NODE: - // m_aNodeType = NodeType::COMMENT_NODE; - pCNode = static_cast< CNode* >(new CComment(*(CDocument*)0, pNode)); - break; - case XML_DOCUMENT_NODE: - // m_aNodeType = NodeType::DOCUMENT_NODE; - pCNode = static_cast< CNode* >(new CDocument( - reinterpret_cast(pNode))); - break; - case XML_DOCUMENT_TYPE_NODE: - case XML_DTD_NODE: - // m_aNodeType = NodeType::DOCUMENT_TYPE_NODE; - pCNode = static_cast< CNode* >(new CDocumentType(*(CDocument*)0, - reinterpret_cast(pNode))); - break; - case XML_DOCUMENT_FRAG_NODE: - // m_aNodeType = NodeType::DOCUMENT_FRAGMENT_NODE; - pCNode = static_cast< CNode* >(new CDocumentFragment(*(CDocument*)0, pNode)); - break; - case XML_NOTATION_NODE: - // m_aNodeType = NodeType::NOTATION_NODE; - pCNode = static_cast< CNode* >(new CNotation(*(CDocument*)0, - reinterpret_cast(pNode))); - break; - case XML_ATTRIBUTE_NODE: - // m_aNodeType = NodeType::ATTRIBUTE_NODE; - pCNode = static_cast< CNode* >(new CAttr(*(CDocument*)0, - reinterpret_cast(pNode))); - break; - // unsupported node types - case XML_HTML_DOCUMENT_NODE: - case XML_ELEMENT_DECL: - case XML_ATTRIBUTE_DECL: - case XML_ENTITY_DECL: - case XML_NAMESPACE_DECL: - default: - break; - } - - if (pCNode != 0) { - bool const bInserted = g_theNodeMap.insert( - nodemap_t::value_type(pNode, - ::std::make_pair(WeakReference(pCNode.get()), - pCNode.get())) - ).second; - OSL_ASSERT(bInserted); - if (!bInserted) { - // if insertion failed, delete new instance and return null - return 0; - } - } - - OSL_ENSURE(pCNode.is(), "no node produced during CNode::getCNode!"); - return pCNode; - } - - xmlNodePtr CNode::getNodePtr(const Reference< XNode >& aNode) - { - try { - CNode *const pNode = GetImplementation(aNode); - if( pNode ) - return pNode->m_aNodePtr; - } - catch(...) {} - return 0; - } - CNode::CNode(CDocument const& rDocument, NodeType const& reNodeType, xmlNodePtr const& rpNode) @@ -316,8 +158,8 @@ namespace DOM void CNode::invalidate() { //remove from list if this wrapper goes away - if (m_aNodePtr != 0) { - CNode::removeCNode(m_aNodePtr, this); + if (m_aNodePtr != 0 && m_xDocument.is()) { + m_xDocument->RemoveCNode(m_aNodePtr, this); } // #i113663#: unlinked nodes will not be freed by xmlFreeDoc if (m_bUnlinked) { @@ -342,15 +184,10 @@ namespace DOM return pCNode; } - ::rtl::Reference< CDocument > CNode::GetOwnerDocument_Impl() + CDocument & CNode::GetOwnerDocument() { - if (0 == m_aNodePtr) { - return 0; - } - ::rtl::Reference< CDocument > const xDoc( - dynamic_cast(CNode::getCNode( - reinterpret_cast(m_aNodePtr->doc)).get())); - return xDoc; + OSL_ASSERT(m_xDocument.is()); + return *m_xDocument; // needs overriding in CDocument! } @@ -443,12 +280,15 @@ namespace DOM /** Adds the node newChild to the end of the list of children of this node. */ - Reference< XNode > CNode::appendChild(const Reference< XNode >& newChild) + Reference< XNode > CNode::appendChild(Reference< XNode > const& xNewChild) throw (RuntimeException, DOMException) { ::rtl::Reference pNode; if (m_aNodePtr != NULL) { - xmlNodePtr cur = CNode::getNodePtr(newChild.get()); + CNode *const pNewChild(CNode::GetImplementation(xNewChild)); + if (!pNewChild) { throw RuntimeException(); } + xmlNodePtr const cur = pNewChild->GetNodePtr(); + if (!cur) { throw RuntimeException(); } // error checks: // from other document @@ -515,11 +355,7 @@ namespace DOM // if res != cur, something was optimized and the newchild-wrapper // should be updated if (res && (cur != res)) { - ::rtl::Reference pCNode(CNode::getCNode(cur)); - OSL_ASSERT(pCNode.is()); - if (pCNode.is()) { - pCNode->invalidate(); // cur has been freed - } + pNewChild->invalidate(); // cur has been freed } // use custom ns cleanup instaead of @@ -527,7 +363,7 @@ namespace DOM // because that will not remove unneeded ns decls _nscleanup(res, m_aNodePtr); - pNode = CNode::getCNode(res); + pNode = GetOwnerDocument().GetCNode(res); } //XXX check for errors @@ -542,7 +378,7 @@ namespace DOM OUString::createFromAscii("DOMNodeInserted")), UNO_QUERY); event->initMutationEvent(OUString::createFromAscii("DOMNodeInserted") , sal_True, sal_False, - Reference< XNode >(CNode::getCNode(m_aNodePtr).get()), + this, OUString(), OUString(), OUString(), (AttrChangeType)0 ); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); @@ -562,7 +398,7 @@ namespace DOM if (0 == m_aNodePtr) { return 0; } - ::rtl::Reference const pNode = CNode::getCNode( + ::rtl::Reference const pNode = GetOwnerDocument().GetCNode( xmlCopyNode(m_aNodePtr, (bDeep) ? 1 : 0)); pNode->m_bUnlinked = true; // not linked yet //XXX check for errors @@ -610,7 +446,7 @@ namespace DOM return 0; } Reference< XNode > const xNode( - CNode::getCNode(m_aNodePtr->children).get()); + GetOwnerDocument().GetCNode(m_aNodePtr->children).get()); return xNode; } @@ -624,7 +460,7 @@ namespace DOM return 0; } Reference< XNode > const xNode( - CNode::getCNode(xmlGetLastChild(m_aNodePtr)).get()); + GetOwnerDocument().GetCNode(xmlGetLastChild(m_aNodePtr)).get()); return xNode; } @@ -674,7 +510,8 @@ namespace DOM if (0 == m_aNodePtr) { return 0; } - Reference< XNode > const xNode(CNode::getCNode(m_aNodePtr->next).get()); + Reference< XNode > const xNode( + GetOwnerDocument().GetCNode(m_aNodePtr->next).get()); return xNode; } @@ -731,7 +568,10 @@ namespace DOM Reference< XDocument > SAL_CALL CNode::getOwnerDocument() throw (RuntimeException) { - Reference< XDocument > const xDoc(GetOwnerDocument_Impl().get()); + if (0 == m_aNodePtr) { + return 0; + } + Reference< XDocument > const xDoc(& GetOwnerDocument()); return xDoc; } @@ -745,7 +585,7 @@ namespace DOM return 0; } Reference< XNode > const xNode( - CNode::getCNode(m_aNodePtr->parent).get()); + GetOwnerDocument().GetCNode(m_aNodePtr->parent).get()); return xNode; } @@ -778,7 +618,7 @@ namespace DOM return 0; } Reference< XNode > const xNode( - CNode::getCNode(m_aNodePtr->prev).get()); + GetOwnerDocument().GetCNode(m_aNodePtr->prev).get()); return xNode; } @@ -819,8 +659,11 @@ namespace DOM throw e; } - xmlNodePtr pRefChild = CNode::getNodePtr(refChild.get()); - xmlNodePtr pNewChild = CNode::getNodePtr(newChild.get()); + CNode *const pNewNode(CNode::GetImplementation(newChild)); + CNode *const pRefNode(CNode::GetImplementation(refChild)); + if (!pNewNode || !pRefNode) { throw RuntimeException(); } + xmlNodePtr const pNewChild(pNewNode->GetNodePtr()); + xmlNodePtr const pRefChild(pRefNode->GetNodePtr()); xmlNodePtr cur = m_aNodePtr->children; //search cild before which to insert @@ -833,8 +676,7 @@ namespace DOM cur->prev = pNewChild; if( pNewChild->prev != NULL) pNewChild->prev->next = pNewChild; - ::rtl::Reference const pNode(CNode::getCNode(pNewChild)); - pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc + pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; } @@ -884,8 +726,8 @@ namespace DOM Reference xReturn( oldChild ); - xmlNodePtr old = CNode::getNodePtr(oldChild); - ::rtl::Reference const pOld( CNode::getCNode(old) ); + ::rtl::Reference const pOld(CNode::GetImplementation(oldChild)); + xmlNodePtr const old = pOld->GetNodePtr(); pOld->m_bUnlinked = true; if( old->type == XML_ATTRIBUTE_NODE ) @@ -915,7 +757,7 @@ namespace DOM OUString::createFromAscii("DOMNodeRemoved")), UNO_QUERY); event->initMutationEvent(OUString::createFromAscii("DOMNodeRemoved"), sal_True, sal_False, - Reference< XNode >(CNode::getCNode(m_aNodePtr).get()), + this, OUString(), OUString(), OUString(), (AttrChangeType)0 ); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); @@ -930,15 +772,16 @@ namespace DOM and returns the oldChild node. */ Reference< XNode > SAL_CALL CNode::replaceChild( - const Reference< XNode >& newChild, const Reference< XNode >& oldChild) + Reference< XNode > const& xNewChild, + Reference< XNode > const& xOldChild) throw (RuntimeException, DOMException) { - if (!oldChild.is()) { + if (!xOldChild.is()) { throw RuntimeException(); } // XXX check node types - if (oldChild->getParentNode() != Reference< XNode >(this)) { + if (xOldChild->getParentNode() != Reference< XNode >(this)) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; @@ -948,8 +791,12 @@ namespace DOM Reference< XNode > aNode = removeChild(oldChild); appendChild(newChild); */ - xmlNodePtr pOld = CNode::getNodePtr(oldChild); - xmlNodePtr pNew = CNode::getNodePtr(newChild); + ::rtl::Reference const pOldNode( + CNode::GetImplementation(xOldChild)); + ::rtl::Reference const pNewNode( + CNode::GetImplementation(xNewChild)); + xmlNodePtr const pOld = pOldNode->GetNodePtr(); + xmlNodePtr const pNew = pNewNode->GetNodePtr(); if( pOld->type == XML_ATTRIBUTE_NODE ) { @@ -963,9 +810,8 @@ namespace DOM xmlAttrPtr pAttr = (xmlAttrPtr)pOld; xmlRemoveProp( pAttr ); - ::rtl::Reference const pOldNode( CNode::getCNode(pOld) ); pOldNode->invalidate(); // freed by xmlRemoveProp - appendChild( newChild ); + appendChild(xNewChild); } else { @@ -991,9 +837,7 @@ namespace DOM pOld->next = NULL; pOld->prev = NULL; pOld->parent = NULL; - ::rtl::Reference const pOldNode(CNode::getCNode(pOld)); pOldNode->m_bUnlinked = true; - ::rtl::Reference const pNewNode(CNode::getCNode(pNew)); pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc } cur = cur->next; @@ -1002,7 +846,7 @@ namespace DOM dispatchSubtreeModified(); - return oldChild; + return xOldChild; } void CNode::dispatchSubtreeModified() @@ -1054,8 +898,8 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { - events::CEventDispatcher & rDispatcher( - m_xDocument->GetEventDispatcher()); + CDocument & rDocument(GetOwnerDocument()); + events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); rDispatcher.addListener(m_aNodePtr, eventType, listener, useCapture); } @@ -1064,17 +908,17 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { - events::CEventDispatcher & rDispatcher( - m_xDocument->GetEventDispatcher()); + CDocument & rDocument(GetOwnerDocument()); + events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); rDispatcher.removeListener(m_aNodePtr, eventType, listener, useCapture); } sal_Bool SAL_CALL CNode::dispatchEvent(const Reference< XEvent >& evt) throw(RuntimeException, EventException) { - events::CEventDispatcher & rDispatcher( - m_xDocument->GetEventDispatcher()); - rDispatcher.dispatchEvent(this, evt); + CDocument & rDocument(GetOwnerDocument()); + events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); + rDispatcher.dispatchEvent(rDocument, m_aNodePtr, this, evt); return sal_True; } -- cgit From 12de6d548b19e4d016e7a7be0980c2d78fe20d17 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:26 +0100 Subject: xmlfix3: #i113682#: unoxml: CDocument gets a member mutex: use it to lock all CNode and derived classes' UNO methods. --- unoxml/source/dom/node.cxx | 104 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 87 insertions(+), 17 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index f16babb3ab2f..501fd134c793 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -142,7 +142,7 @@ namespace DOM } - CNode::CNode(CDocument const& rDocument, + CNode::CNode(CDocument const& rDocument, ::osl::Mutex const& rMutex, NodeType const& reNodeType, xmlNodePtr const& rpNode) : m_bUnlinked(false) , m_aNodeType(reNodeType) @@ -151,6 +151,7 @@ namespace DOM // (but not if this is a document; that would create a leak!) , m_xDocument( (m_aNodePtr->type != XML_DOCUMENT_NODE) ? &const_cast(rDocument) : 0 ) + , m_rMutex(const_cast< ::osl::Mutex & >(rMutex)) { OSL_ASSERT(m_aNodePtr); } @@ -170,7 +171,13 @@ namespace DOM CNode::~CNode() { - invalidate(); + // if this is the document itself, the mutex is already freed! + if (NodeType_DOCUMENT_NODE == m_aNodeType) { + invalidate(); + } else { + ::osl::MutexGuard const g(m_rMutex); + invalidate(); // other nodes are still alive so must lock mutex + } } CNode * @@ -266,13 +273,14 @@ namespace DOM } } - void SAL_CALL CNode::saxify( - const Reference< XDocumentHandler >& i_xHandler) { + void CNode::saxify(const Reference< XDocumentHandler >& i_xHandler) + { if (!i_xHandler.is()) throw RuntimeException(); // default: do nothing } - void SAL_CALL CNode::fastSaxify(Context& io_rContext) { + void CNode::fastSaxify(Context& io_rContext) + { if (!io_rContext.mxDocHandler.is()) throw RuntimeException(); // default: do nothing } @@ -280,9 +288,12 @@ namespace DOM /** Adds the node newChild to the end of the list of children of this node. */ - Reference< XNode > CNode::appendChild(Reference< XNode > const& xNewChild) + Reference< XNode > SAL_CALL CNode::appendChild( + Reference< XNode > const& xNewChild) throw (RuntimeException, DOMException) { + ::osl::ClearableMutexGuard guard(m_rMutex); + ::rtl::Reference pNode; if (m_aNodePtr != NULL) { CNode *const pNewChild(CNode::GetImplementation(xNewChild)); @@ -380,8 +391,12 @@ namespace DOM , sal_True, sal_False, this, OUString(), OUString(), OUString(), (AttrChangeType)0 ); - dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); + // the following dispatch functions use only UNO interfaces + // and call event listeners, so release mutex to prevent deadlocks. + guard.clear(); + + dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); // dispatch subtree modified for this node dispatchSubtreeModified(); } @@ -392,9 +407,11 @@ namespace DOM Returns a duplicate of this node, i.e., serves as a generic copy constructor for nodes. */ - Reference< XNode > CNode::cloneNode(sal_Bool bDeep) + Reference< XNode > SAL_CALL CNode::cloneNode(sal_Bool bDeep) throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -409,9 +426,10 @@ namespace DOM A NamedNodeMap containing the attributes of this node (if it is an Element) or null otherwise. */ - Reference< XNamedNodeMap > CNode::getAttributes() + Reference< XNamedNodeMap > SAL_CALL CNode::getAttributes() throw (RuntimeException) { + // return empty reference // only element node may override this impl return Reference< XNamedNodeMap>(); @@ -426,9 +444,11 @@ namespace DOM /** A NodeList that contains all children of this node. */ - Reference< XNodeList > CNode::getChildNodes() + Reference< XNodeList > SAL_CALL CNode::getChildNodes() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -439,9 +459,11 @@ namespace DOM /** The first child of this node. */ - Reference< XNode > CNode::getFirstChild() + Reference< XNode > SAL_CALL CNode::getFirstChild() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -456,6 +478,8 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getLastChild() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -490,6 +514,8 @@ namespace DOM OUString SAL_CALL CNode::getNamespaceURI() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + OUString aURI; if (m_aNodePtr != NULL && (m_aNodePtr->type == XML_ELEMENT_NODE || m_aNodePtr->type == XML_ATTRIBUTE_NODE) && @@ -507,6 +533,8 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getNextSibling() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -549,6 +577,8 @@ namespace DOM NodeType SAL_CALL CNode::getNodeType() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + return m_aNodeType; } @@ -568,6 +598,8 @@ namespace DOM Reference< XDocument > SAL_CALL CNode::getOwnerDocument() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -581,6 +613,8 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getParentNode() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -595,6 +629,8 @@ namespace DOM OUString SAL_CALL CNode::getPrefix() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + OUString aPrefix; if (m_aNodePtr != NULL && (m_aNodePtr->type == XML_ELEMENT_NODE || m_aNodePtr->type == XML_ATTRIBUTE_NODE) && @@ -614,6 +650,8 @@ namespace DOM Reference< XNode > SAL_CALL CNode::getPreviousSibling() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + if (0 == m_aNodePtr) { return 0; } @@ -628,6 +666,8 @@ namespace DOM sal_Bool SAL_CALL CNode::hasAttributes() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + return (m_aNodePtr != NULL && m_aNodePtr->properties != NULL); } @@ -637,6 +677,8 @@ namespace DOM sal_Bool SAL_CALL CNode::hasChildNodes() throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + return (m_aNodePtr != NULL && m_aNodePtr->children != NULL); } @@ -647,7 +689,6 @@ namespace DOM const Reference< XNode >& newChild, const Reference< XNode >& refChild) throw (RuntimeException, DOMException) { - if (newChild->getOwnerDocument() != getOwnerDocument()) { DOMException e; e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; @@ -659,6 +700,8 @@ namespace DOM throw e; } + ::osl::MutexGuard const g(m_rMutex); + CNode *const pNewNode(CNode::GetImplementation(newChild)); CNode *const pRefNode(CNode::GetImplementation(refChild)); if (!pNewNode || !pRefNode) { throw RuntimeException(); } @@ -724,6 +767,8 @@ namespace DOM throw e; } + ::osl::ClearableMutexGuard guard(m_rMutex); + Reference xReturn( oldChild ); ::rtl::Reference const pOld(CNode::GetImplementation(oldChild)); @@ -759,8 +804,12 @@ namespace DOM sal_False, this, OUString(), OUString(), OUString(), (AttrChangeType)0 ); - dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); + // the following dispatch functions use only UNO interfaces + // and call event listeners, so release mutex to prevent deadlocks. + guard.clear(); + + dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); // subtree modofied for this node dispatchSubtreeModified(); } @@ -787,6 +836,8 @@ namespace DOM throw e; } + ::osl::ClearableMutexGuard guard(m_rMutex); + /* Reference< XNode > aNode = removeChild(oldChild); appendChild(newChild); @@ -844,6 +895,7 @@ namespace DOM } } + guard.clear(); // release for calling event handlers dispatchSubtreeModified(); return xOldChild; @@ -851,12 +903,15 @@ namespace DOM void CNode::dispatchSubtreeModified() { + // only uses UNO interfaces => needs no mutex + // dispatch DOMSubtreeModified // target is _this_ node Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); Reference< XMutationEvent > event(docevent->createEvent( OUString::createFromAscii("DOMSubtreeModified")), UNO_QUERY); - event->initMutationEvent(OUString::createFromAscii("DOMSubtreeModified"), sal_True, + event->initMutationEvent( + OUString::createFromAscii("DOMSubtreeModified"), sal_True, sal_False, Reference< XNode >(), OUString(), OUString(), OUString(), (AttrChangeType)0 ); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); @@ -881,6 +936,8 @@ namespace DOM void SAL_CALL CNode::setPrefix(const OUString& prefix) throw (RuntimeException, DOMException) { + ::osl::MutexGuard const g(m_rMutex); + OString o1 = OUStringToOString(prefix, RTL_TEXTENCODING_UTF8); xmlChar *pBuf = (xmlChar*)o1.getStr(); // XXX copy buf? @@ -898,6 +955,8 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + CDocument & rDocument(GetOwnerDocument()); events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); rDispatcher.addListener(m_aNodePtr, eventType, listener, useCapture); @@ -908,6 +967,8 @@ namespace DOM sal_Bool useCapture) throw (RuntimeException) { + ::osl::MutexGuard const g(m_rMutex); + CDocument & rDocument(GetOwnerDocument()); events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); rDispatcher.removeListener(m_aNodePtr, eventType, listener, useCapture); @@ -916,9 +977,18 @@ namespace DOM sal_Bool SAL_CALL CNode::dispatchEvent(const Reference< XEvent >& evt) throw(RuntimeException, EventException) { - CDocument & rDocument(GetOwnerDocument()); - events::CEventDispatcher & rDispatcher(rDocument.GetEventDispatcher()); - rDispatcher.dispatchEvent(rDocument, m_aNodePtr, this, evt); + CDocument * pDocument; + events::CEventDispatcher * pDispatcher; + xmlNodePtr pNode; + { + ::osl::MutexGuard const g(m_rMutex); + + pDocument = & GetOwnerDocument(); + pDispatcher = & pDocument->GetEventDispatcher(); + pNode = m_aNodePtr; + } + // this calls event listeners, do not call with locked mutex + pDispatcher->dispatchEvent(*pDocument, m_rMutex, pNode, this, evt); return sal_True; } -- cgit From dd995cea7b9a262d51b94d536817e5aa2950f1cb Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:26 +0100 Subject: xmlfix3: #i113682#: unoxml: use CDocument mutex in misc classes --- unoxml/source/dom/node.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 501fd134c793..55978a507a67 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -452,7 +452,7 @@ namespace DOM if (0 == m_aNodePtr) { return 0; } - Reference< XNodeList > const xNodeList(new CChildList(this)); + Reference< XNodeList > const xNodeList(new CChildList(this, m_rMutex)); return xNodeList; } -- cgit From a877dd2c109e8e76923376b18daca101edb4a5e2 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:27 +0100 Subject: xmlfix3: unoxml: some cleanup in CNode... --- unoxml/source/dom/node.cxx | 258 ++++++++++++++++++++++----------------------- 1 file changed, 124 insertions(+), 134 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 55978a507a67..a25190a65117 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -198,7 +198,8 @@ namespace DOM } - static void _nsexchange(const xmlNodePtr aNode, xmlNsPtr oldNs, xmlNsPtr newNs) + static void lcl_nsexchange( + xmlNodePtr const aNode, xmlNsPtr const oldNs, xmlNsPtr const newNs) { // recursively exchange any references to oldNs with references to newNs xmlNodePtr cur = aNode; @@ -215,13 +216,13 @@ namespace DOM curAttr->ns = newNs; curAttr = curAttr->next; } - _nsexchange(cur->children, oldNs, newNs); + lcl_nsexchange(cur->children, oldNs, newNs); } cur = cur->next; } } - /*static*/ void _nscleanup(const xmlNodePtr aNode, const xmlNodePtr aParent) + /*static*/ void nscleanup(const xmlNodePtr aNode, const xmlNodePtr aParent) { xmlNodePtr cur = aNode; @@ -243,7 +244,7 @@ namespace DOM while (cur != NULL) { - _nscleanup(cur->children, cur); + nscleanup(cur->children, cur); if (cur->ns != NULL) { xmlNsPtr ns = xmlSearchNs(cur->doc, aParent, cur->ns->prefix); @@ -258,7 +259,7 @@ namespace DOM { // reconnect ns pointers in sub-tree to newly found ns before // removing redundant nsdecl to prevent dangling pointers. - _nsexchange(cur, curDef, ns); + lcl_nsexchange(cur, curDef, ns); *refp = curDef->next; xmlFreeNs(curDef); curDef = *refp; @@ -294,112 +295,119 @@ namespace DOM { ::osl::ClearableMutexGuard guard(m_rMutex); - ::rtl::Reference pNode; - if (m_aNodePtr != NULL) { - CNode *const pNewChild(CNode::GetImplementation(xNewChild)); - if (!pNewChild) { throw RuntimeException(); } - xmlNodePtr const cur = pNewChild->GetNodePtr(); - if (!cur) { throw RuntimeException(); } + if (0 == m_aNodePtr) { return 0; } - // error checks: - // from other document - if (cur->doc != m_aNodePtr->doc) { - DOMException e; - e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; - throw e; - } - // same node - if (cur == m_aNodePtr) { - DOMException e; - e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; - throw e; - } - // already has parant and is not attribute - if (cur->parent != NULL && cur->type != XML_ATTRIBUTE_NODE) { - DOMException e; - e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; - throw e; - } + CNode *const pNewChild(CNode::GetImplementation(xNewChild)); + if (!pNewChild) { throw RuntimeException(); } + xmlNodePtr const cur = pNewChild->GetNodePtr(); + if (!cur) { throw RuntimeException(); } - // check whether this is an attribute node so we remove it's - // carrier node if it has one - xmlNodePtr res = NULL; - if (cur->type == XML_ATTRIBUTE_NODE) - { - if (cur->parent != NULL) - { - if (m_aNodePtr->type != XML_ELEMENT_NODE || - strcmp((char*)cur->parent->name, "__private") != 0) - { - DOMException e; - e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; - throw e; - } + // error checks: + // from other document + if (cur->doc != m_aNodePtr->doc) { + DOMException e; + e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; + throw e; + } + // same node + if (cur == m_aNodePtr) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } + // already has parent and is not attribute + if (cur->parent != NULL && cur->type != XML_ATTRIBUTE_NODE) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } - xmlNsPtr pAttrNs = cur->ns; - xmlNsPtr pParentNs = xmlSearchNs(m_aNodePtr->doc, m_aNodePtr, pAttrNs->prefix); - if (pParentNs == NULL || strcmp((char*)pParentNs->href, (char*)pAttrNs->href) != 0) - pParentNs = xmlNewNs(m_aNodePtr, pAttrNs->href, pAttrNs->prefix); + // check whether this is an attribute node so we remove it's + // carrier node if it has one + xmlNodePtr res = NULL; + if (cur->type == XML_ATTRIBUTE_NODE) + { + if (cur->parent != NULL) { + if (m_aNodePtr->type != XML_ELEMENT_NODE || + strcmp((char*)cur->parent->name, "__private") != 0) + { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } - if (cur->children != NULL) - res = (xmlNodePtr)xmlNewNsProp(m_aNodePtr, pParentNs, cur->name, cur->children->content); - else - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, cur->name, (xmlChar*) ""); + xmlNsPtr pAttrNs = cur->ns; + xmlNsPtr pParentNs = + xmlSearchNs(m_aNodePtr->doc, m_aNodePtr, pAttrNs->prefix); + if (pParentNs == NULL || + strcmp((char*)pParentNs->href, (char*)pAttrNs->href) != 0) + { + pParentNs = + xmlNewNs(m_aNodePtr, pAttrNs->href, pAttrNs->prefix); + } - xmlFreeNode(cur->parent); - cur->parent = NULL; + if (cur->children != NULL) { + res = (xmlNodePtr)xmlNewNsProp(m_aNodePtr, + pParentNs, cur->name, cur->children->content); + } else { + res = (xmlNodePtr)xmlNewProp(m_aNodePtr, + cur->name, (xmlChar*) ""); } - else - { - if (cur->children != NULL) - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, cur->name, cur->children->content); - else - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, cur->name, (xmlChar*) ""); + + xmlFreeNode(cur->parent); + cur->parent = NULL; + } else { + if (cur->children != NULL) { + res = (xmlNodePtr)xmlNewProp(m_aNodePtr, + cur->name, cur->children->content); + } else { + res = (xmlNodePtr)xmlNewProp(m_aNodePtr, + cur->name, (xmlChar*) ""); } } - else - { - res = xmlAddChild(m_aNodePtr, cur); - } + } + else + { + res = xmlAddChild(m_aNodePtr, cur); + } - // libxml can do optimizations, when appending nodes. - // if res != cur, something was optimized and the newchild-wrapper - // should be updated - if (res && (cur != res)) { - pNewChild->invalidate(); // cur has been freed - } + // libxml can do optimizations, when appending nodes. + // if res != cur, something was optimized and the newchild-wrapper + // should be updated + if (res && (cur != res)) { + pNewChild->invalidate(); // cur has been freed + } - // use custom ns cleanup instaead of - // xmlReconciliateNs(m_aNodePtr->doc, m_aNodePtr); + // use custom ns cleanup instead of + // xmlReconciliateNs(m_aNodePtr->doc, m_aNodePtr); // because that will not remove unneeded ns decls - _nscleanup(res, m_aNodePtr); + nscleanup(res, m_aNodePtr); - pNode = GetOwnerDocument().GetCNode(res); - } + ::rtl::Reference const pNode = GetOwnerDocument().GetCNode(res); + + if (!pNode.is()) { return 0; } //XXX check for errors // dispatch DOMNodeInserted event, target is the new node // this node is the related node // does bubble - if (pNode.is()) - { - pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc - Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); - Reference< XMutationEvent > event(docevent->createEvent( - OUString::createFromAscii("DOMNodeInserted")), UNO_QUERY); - event->initMutationEvent(OUString::createFromAscii("DOMNodeInserted") - , sal_True, sal_False, - this, - OUString(), OUString(), OUString(), (AttrChangeType)0 ); - - // the following dispatch functions use only UNO interfaces - // and call event listeners, so release mutex to prevent deadlocks. - guard.clear(); - - dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); - // dispatch subtree modified for this node - dispatchSubtreeModified(); - } + pNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc + Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); + Reference< XMutationEvent > event(docevent->createEvent( + OUString::createFromAscii("DOMNodeInserted")), UNO_QUERY); + event->initMutationEvent(OUString::createFromAscii("DOMNodeInserted") + , sal_True, sal_False, + this, + OUString(), OUString(), OUString(), (AttrChangeType)0 ); + + // the following dispatch functions use only UNO interfaces + // and call event listeners, so release mutex to prevent deadlocks. + guard.clear(); + + dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); + // dispatch subtree modified for this node + dispatchSubtreeModified(); + return pNode.get(); } @@ -429,16 +437,8 @@ namespace DOM Reference< XNamedNodeMap > SAL_CALL CNode::getAttributes() throw (RuntimeException) { - - // return empty reference - // only element node may override this impl + // return empty reference; only element node may override this impl return Reference< XNamedNodeMap>(); - - // get all children that are attributes - /* --> CElement - Reference< NamedNodeMap > aNodeMap(new AttributeNamedNodeMap(m_aNodePtr), UNO_QUERY); - return aNodeMap; - */ } /** @@ -494,17 +494,8 @@ namespace DOM OUString SAL_CALL CNode::getLocalName() throw (RuntimeException) { - OUString aName; - /* - --> Element / Attribute - if(m_aNodePtr != NULL && (m_aNodeType == NodeType::ATTRIBUTE_NODE - || m_aNodeType == NodeType::ELEMENT_NODE)) - { - aName = OUString(m_aNodePtr->name, RTL_TEXTENCODING_UTF8); - } - //XXX error checking - */ - return aName; + // see CElement/CAttr + return ::rtl::OUString(); } @@ -709,7 +700,7 @@ namespace DOM xmlNodePtr const pRefChild(pRefNode->GetNodePtr()); xmlNodePtr cur = m_aNodePtr->children; - //search cild before which to insert + //search child before which to insert while (cur != NULL) { if (cur == pRefChild) { @@ -795,24 +786,23 @@ namespace DOM * Cancelable: No * Context Info: relatedNode holds the parent node */ - if (oldChild.is()) - { - Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); - Reference< XMutationEvent > event(docevent->createEvent( - OUString::createFromAscii("DOMNodeRemoved")), UNO_QUERY); - event->initMutationEvent(OUString::createFromAscii("DOMNodeRemoved"), sal_True, - sal_False, - this, - OUString(), OUString(), OUString(), (AttrChangeType)0 ); - - // the following dispatch functions use only UNO interfaces - // and call event listeners, so release mutex to prevent deadlocks. - guard.clear(); - - dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); - // subtree modofied for this node - dispatchSubtreeModified(); - } + Reference< XDocumentEvent > docevent(getOwnerDocument(), UNO_QUERY); + Reference< XMutationEvent > event(docevent->createEvent( + OUString::createFromAscii("DOMNodeRemoved")), UNO_QUERY); + event->initMutationEvent(OUString::createFromAscii("DOMNodeRemoved"), + sal_True, + sal_False, + this, + OUString(), OUString(), OUString(), (AttrChangeType)0 ); + + // the following dispatch functions use only UNO interfaces + // and call event listeners, so release mutex to prevent deadlocks. + guard.clear(); + + dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); + // subtree modofied for this node + dispatchSubtreeModified(); + return xReturn; } @@ -920,7 +910,7 @@ namespace DOM /** The value of this node, depending on its type; see the table above. */ - void SAL_CALL CNode::setNodeValue(const OUString& /*nodeValue*/) + void SAL_CALL CNode::setNodeValue(const OUString& /*nodeValue*/) throw (RuntimeException, DOMException) { // use specific node implememntation -- cgit From b544b4a492a23b0d00f65306a2839b1ed7cd7355 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:27 +0100 Subject: xmlfix3: unoxml: CNode fixes: add some additional argument checks. fix mess in CNode::setPrefix(). --- unoxml/source/dom/node.cxx | 76 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 62 insertions(+), 14 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index a25190a65117..54645e35ad78 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -30,6 +30,8 @@ #include #include +#include + #include #include @@ -378,6 +380,8 @@ namespace DOM pNewChild->invalidate(); // cur has been freed } + if (!res) { return 0; } + // use custom ns cleanup instead of // xmlReconciliateNs(m_aNodePtr->doc, m_aNodePtr); // because that will not remove unneeded ns decls @@ -386,7 +390,6 @@ namespace DOM ::rtl::Reference const pNode = GetOwnerDocument().GetCNode(res); if (!pNode.is()) { return 0; } - //XXX check for errors // dispatch DOMNodeInserted event, target is the new node // this node is the related node @@ -425,8 +428,8 @@ namespace DOM } ::rtl::Reference const pNode = GetOwnerDocument().GetCNode( xmlCopyNode(m_aNodePtr, (bDeep) ? 1 : 0)); + if (!pNode.is()) { return 0; } pNode->m_bUnlinked = true; // not linked yet - //XXX check for errors return pNode.get(); } @@ -680,6 +683,8 @@ namespace DOM const Reference< XNode >& newChild, const Reference< XNode >& refChild) throw (RuntimeException, DOMException) { + if (!newChild.is() || !refChild.is()) { throw RuntimeException(); } + if (newChild->getOwnerDocument() != getOwnerDocument()) { DOMException e; e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; @@ -698,6 +703,21 @@ namespace DOM if (!pNewNode || !pRefNode) { throw RuntimeException(); } xmlNodePtr const pNewChild(pNewNode->GetNodePtr()); xmlNodePtr const pRefChild(pRefNode->GetNodePtr()); + if (!pNewChild || !pRefChild) { throw RuntimeException(); } + + if (pNewChild == m_aNodePtr) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } + // already has parent and is not attribute + if (pNewChild->parent != NULL && pNewChild->type != XML_ATTRIBUTE_NODE) + { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } + xmlNodePtr cur = m_aNodePtr->children; //search child before which to insert @@ -745,14 +765,20 @@ namespace DOM Removes the child node indicated by oldChild from the list of children, and returns it. */ - Reference< XNode > SAL_CALL CNode::removeChild(const Reference< XNode >& oldChild) + Reference< XNode > SAL_CALL + CNode::removeChild(const Reference< XNode >& xOldChild) throw (RuntimeException, DOMException) { - if (!oldChild.is()) { + if (!xOldChild.is()) { throw RuntimeException(); } - if (oldChild->getParentNode() != Reference< XNode >(this)) { + if (xOldChild->getOwnerDocument() != getOwnerDocument()) { + DOMException e; + e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; + throw e; + } + if (xOldChild->getParentNode() != Reference< XNode >(this)) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; @@ -760,15 +786,18 @@ namespace DOM ::osl::ClearableMutexGuard guard(m_rMutex); - Reference xReturn( oldChild ); + if (!m_aNodePtr) { throw RuntimeException(); } - ::rtl::Reference const pOld(CNode::GetImplementation(oldChild)); + Reference xReturn( xOldChild ); + + ::rtl::Reference const pOld(CNode::GetImplementation(xOldChild)); + if (!pOld.is()) { throw RuntimeException(); } xmlNodePtr const old = pOld->GetNodePtr(); - pOld->m_bUnlinked = true; + if (!old) { throw RuntimeException(); } if( old->type == XML_ATTRIBUTE_NODE ) { - xmlAttrPtr pAttr = (xmlAttrPtr) old; + xmlAttrPtr pAttr = reinterpret_cast(old); xmlRemoveProp( pAttr ); pOld->invalidate(); // freed by xmlRemoveProp xReturn.clear(); @@ -776,6 +805,7 @@ namespace DOM else { xmlUnlinkNode(old); + pOld->m_bUnlinked = true; } /*DOMNodeRemoved @@ -800,7 +830,7 @@ namespace DOM guard.clear(); dispatchEvent(Reference< XEvent >(event, UNO_QUERY)); - // subtree modofied for this node + // subtree modified for this node dispatchSubtreeModified(); return xReturn; @@ -815,11 +845,16 @@ namespace DOM Reference< XNode > const& xOldChild) throw (RuntimeException, DOMException) { - if (!xOldChild.is()) { + if (!xOldChild.is() || !xNewChild.is()) { throw RuntimeException(); } // XXX check node types + if (xNewChild->getOwnerDocument() != getOwnerDocument()) { + DOMException e; + e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR; + throw e; + } if (xOldChild->getParentNode() != Reference< XNode >(this)) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; @@ -836,8 +871,22 @@ namespace DOM CNode::GetImplementation(xOldChild)); ::rtl::Reference const pNewNode( CNode::GetImplementation(xNewChild)); + if (!pOldNode.is() || !pNewNode.is()) { throw RuntimeException(); } xmlNodePtr const pOld = pOldNode->GetNodePtr(); xmlNodePtr const pNew = pNewNode->GetNodePtr(); + if (!pOld || !pNew) { throw RuntimeException(); } + + if (pNew == m_aNodePtr) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } + // already has parent and is not attribute + if (pNew->parent != NULL && pNew->type != XML_ATTRIBUTE_NODE) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } if( pOld->type == XML_ATTRIBUTE_NODE ) { @@ -930,11 +979,10 @@ namespace DOM OString o1 = OUStringToOString(prefix, RTL_TEXTENCODING_UTF8); xmlChar *pBuf = (xmlChar*)o1.getStr(); - // XXX copy buf? - // XXX free old string? (leak?) if (m_aNodePtr != NULL && m_aNodePtr->ns != NULL) { - m_aNodePtr->ns->prefix = pBuf; + xmlFree(const_cast(m_aNodePtr->ns->prefix)); + m_aNodePtr->ns->prefix = xmlStrdup(pBuf); } } -- cgit From d76639d5d833a05a3181f7293e38a250dc6c1299 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 19 Jan 2011 20:27:28 +0100 Subject: xmlfix3: unoxml: eradicate the bizarre concept of "carrier nodes": add namespace data member to CAttr. CAttr overrides getPrefix(), setPrefix(), getNamespaceURI(). CDocument::createAttributeNS() uses this new CAttr member instead of creating a dummy carrier element. CElement::setAttributeNode_Impl_Lock() and CNode::appendChild() do not free the no longer existing dummy carrier element. CNode::insertBefore() calls appendChild() for attributes instead of ignoring namespace. CNode::appendChild() does not invalidate attributes, because they are copied. --- unoxml/source/dom/node.cxx | 82 ++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 51 deletions(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 54645e35ad78..68ceb9bfa3e4 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -43,6 +43,7 @@ #include #include +#include #include #include "../events/eventdispatcher.hxx" @@ -317,67 +318,40 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } - // already has parent and is not attribute - if (cur->parent != NULL && cur->type != XML_ATTRIBUTE_NODE) { + if (cur->parent != NULL) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } - // check whether this is an attribute node so we remove it's - // carrier node if it has one + // check whether this is an attribute node; it needs special handling xmlNodePtr res = NULL; if (cur->type == XML_ATTRIBUTE_NODE) { - if (cur->parent != NULL) { - if (m_aNodePtr->type != XML_ELEMENT_NODE || - strcmp((char*)cur->parent->name, "__private") != 0) - { - DOMException e; - e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; - throw e; - } - - xmlNsPtr pAttrNs = cur->ns; - xmlNsPtr pParentNs = - xmlSearchNs(m_aNodePtr->doc, m_aNodePtr, pAttrNs->prefix); - if (pParentNs == NULL || - strcmp((char*)pParentNs->href, (char*)pAttrNs->href) != 0) - { - pParentNs = - xmlNewNs(m_aNodePtr, pAttrNs->href, pAttrNs->prefix); - } - - if (cur->children != NULL) { - res = (xmlNodePtr)xmlNewNsProp(m_aNodePtr, - pParentNs, cur->name, cur->children->content); - } else { - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, - cur->name, (xmlChar*) ""); - } - - xmlFreeNode(cur->parent); - cur->parent = NULL; + xmlChar const*const pChildren((cur->children) + ? cur->children->content + : reinterpret_cast("")); + CAttr *const pCAttr(dynamic_cast(pNewChild)); + if (!pCAttr) { throw RuntimeException(); } + xmlNsPtr const pNs( pCAttr->GetNamespace(m_aNodePtr) ); + if (pNs) { + res = reinterpret_cast( + xmlNewNsProp(m_aNodePtr, pNs, cur->name, pChildren)); } else { - if (cur->children != NULL) { - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, - cur->name, cur->children->content); - } else { - res = (xmlNodePtr)xmlNewProp(m_aNodePtr, - cur->name, (xmlChar*) ""); - } + res = reinterpret_cast( + xmlNewProp(m_aNodePtr, cur->name, pChildren)); } } else { res = xmlAddChild(m_aNodePtr, cur); - } - // libxml can do optimizations, when appending nodes. - // if res != cur, something was optimized and the newchild-wrapper - // should be updated - if (res && (cur != res)) { - pNewChild->invalidate(); // cur has been freed + // libxml can do optimization when appending nodes. + // if res != cur, something was optimized and the newchild-wrapper + // should be updated + if (res && (cur != res)) { + pNewChild->invalidate(); // cur has been freed + } } if (!res) { return 0; } @@ -696,7 +670,7 @@ namespace DOM throw e; } - ::osl::MutexGuard const g(m_rMutex); + ::osl::ClearableMutexGuard guard(m_rMutex); CNode *const pNewNode(CNode::GetImplementation(newChild)); CNode *const pRefNode(CNode::GetImplementation(refChild)); @@ -710,14 +684,20 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } - // already has parent and is not attribute - if (pNewChild->parent != NULL && pNewChild->type != XML_ATTRIBUTE_NODE) + // already has parent + if (pNewChild->parent != NULL) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } + // attributes are unordered anyway, so just do appendChild + if (XML_ATTRIBUTE_NODE == pNewChild->type) { + guard.clear(); + return appendChild(newChild); + } + xmlNodePtr cur = m_aNodePtr->children; //search child before which to insert @@ -881,8 +861,8 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } - // already has parent and is not attribute - if (pNew->parent != NULL && pNew->type != XML_ATTRIBUTE_NODE) { + // already has parent + if (pNew->parent != NULL) { DOMException e; e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; -- cgit From c5db3b93ee1058bd20ebcde2e757b52b9a67b74a Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Thu, 10 Feb 2011 16:45:02 +0100 Subject: xmlfix3: unoxml: prevent invalid child-parent relationships: new method CNode::IsChildTypeAllowed(NodeType). use it in appendChild(), insertBefore(), replaceChild(). --- unoxml/source/dom/node.cxx | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 68ceb9bfa3e4..4786606c16c0 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -289,6 +289,12 @@ namespace DOM // default: do nothing } + bool CNode::IsChildTypeAllowed(NodeType const /*nodeType*/) + { + // default: no children allowed + return false; + } + /** Adds the node newChild to the end of the list of children of this node. */ @@ -323,6 +329,11 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } + if (!IsChildTypeAllowed(pNewChild->m_aNodeType)) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } // check whether this is an attribute node; it needs special handling xmlNodePtr res = NULL; @@ -691,6 +702,11 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } + if (!IsChildTypeAllowed(pNewNode->m_aNodeType)) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } // attributes are unordered anyway, so just do appendChild if (XML_ATTRIBUTE_NODE == pNewChild->type) { @@ -828,7 +844,6 @@ namespace DOM if (!xOldChild.is() || !xNewChild.is()) { throw RuntimeException(); } - // XXX check node types if (xNewChild->getOwnerDocument() != getOwnerDocument()) { DOMException e; @@ -867,6 +882,11 @@ namespace DOM e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; throw e; } + if (!IsChildTypeAllowed(pNewNode->m_aNodeType)) { + DOMException e; + e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR; + throw e; + } if( pOld->type == XML_ATTRIBUTE_NODE ) { -- cgit From 141773454451569127d388a1f3fd1fdc65f5554a Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Thu, 10 Feb 2011 16:45:03 +0100 Subject: xmlfix3: unoxml: fix various bugs uncovered by complex test: CAttr::getSpecified() should return true. CAttr::getValue() and CElement::setAttributeNode() could crash if no children. CAttributesMap::*NS() methods ignored namespaceURI parameter. CDATASection::getNodeValue() forwarded to wrong base class. CDocument needs to override cloneNode(). CDocument::importNode(), CDocumentBuilder::parseURI() deref null argument. CElement::getAttributes() should return an empty map. CElementList could be created for null element. CNode::insertBefore() did not set parent/children pointers. CNode::setPrefix() should only work on element/attribute. CXPathAPI had inverted check for null argument (xmlfix3 regression). --- unoxml/source/dom/node.cxx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index 4786606c16c0..f134e79230f6 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -724,9 +724,16 @@ namespace DOM pNewChild->next = cur; pNewChild->prev = cur->prev; cur->prev = pNewChild; - if( pNewChild->prev != NULL) + if (pNewChild->prev != NULL) { pNewChild->prev->next = pNewChild; + } + pNewChild->parent = cur->parent; + if (pNewChild->parent->children == cur) { + pNewChild->parent->children = pNewChild; + } + // do not update parent->last here! pNewNode->m_bUnlinked = false; // will be deleted by xmlFreeDoc + break; } cur = cur->next; } @@ -977,6 +984,14 @@ namespace DOM { ::osl::MutexGuard const g(m_rMutex); + if ((0 == m_aNodePtr) || + ((m_aNodePtr->type != XML_ELEMENT_NODE) && + (m_aNodePtr->type != XML_ATTRIBUTE_NODE))) + { + DOMException e; + e.Code = DOMExceptionType_NO_MODIFICATION_ALLOWED_ERR; + throw e; + } OString o1 = OUStringToOString(prefix, RTL_TEXTENCODING_UTF8); xmlChar *pBuf = (xmlChar*)o1.getStr(); if (m_aNodePtr != NULL && m_aNodePtr->ns != NULL) -- cgit From 001145d71e0592bdf90a80f33d1abb32846aae21 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Thu, 10 Feb 2011 16:45:05 +0100 Subject: xmlfix3: #i113683#: unoxml: all unimplemented methods assert --- unoxml/source/dom/node.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'unoxml/source/dom/node.cxx') diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx index f134e79230f6..fb95fa6ade02 100644 --- a/unoxml/source/dom/node.cxx +++ b/unoxml/source/dom/node.cxx @@ -747,7 +747,7 @@ namespace DOM sal_Bool SAL_CALL CNode::isSupported(const OUString& /*feature*/, const OUString& /*ver*/) throw (RuntimeException) { - // XXX + OSL_ENSURE(false, "CNode::isSupported: not implemented (#i113683#)"); return sal_False; } @@ -762,6 +762,7 @@ namespace DOM throw (RuntimeException) { //XXX combine adjacent text nodes and remove empty ones + OSL_ENSURE(false, "CNode::normalize: not implemented (#i113683#)"); } /** -- cgit