[PATCH 1/3] msxml3: Update orphant list in put_documentElement.

[Jacek Caban]

Michael Karcher wrote:
> Am Donnerstag, den 16.10.2008, 13:55 -0500 schrieb Jacek Caban:
> @@ -932,9 +933,17 @@ static HRESULT WINAPI domdoc_put_documentElement(
>          return hr;
>  
>      xmlNode = impl_from_IXMLDOMNode( elementNode );
> -    xmlDocSetRootElement( get_doc(This), xmlNode->node);
> +
> +    if(!xmlNode->node->parent)
> +        if(xmldoc_remove_orphan(xmlNode->node->doc, xmlNode->node) != S_OK)
> +            WARN("%p is not an orphan of %p\n", xmlNode->node->doc, xmlNode->node);
> +
> +    oldRoot = xmlDocSetRootElement( get_doc(This), xmlNode->node);
>      IXMLDOMNode_Release( elementNode );
>  
> +    if(oldRoot)
> +        xmldoc_add_orphan(oldRoot->doc, oldRoot);
> +
>      return S_OK;
>  }
>
> This patch is against my intention of the orphan list. The orphan list
> should contain nodes that are associated to the document, but are not in
> the tree (i.e. not an descendant of the document root element). The
> orphan list is used to not forget to free these nodes (that logically
> reference the document and have to keep the document alive) when the
> document itself (i.e. the root document element) is freed. Now oldRoot
> already *is* the root element, that is oldRoot->doc == oldRoot. Placeing
> the element itself on the orphan list causes it to be freed when freeing
> the orphans and a second time (some lines later) when the document
> element is freed.
>
> So, I doubt this patch is correct.
>   

This patch fixes regression in Photoshop CS4 installer caused by your 
patch. The double free is exactly what happened with your patch and my 
patch fixes it. The new root has to be removed from orphan list because 
it is added to DOM tree. Otherwise we'd free it two times. Also oldRoot 
is no longer in DOM tree, so it should be added to orphan list to be 
freed later.


Jacek