On 5/6/2021 2:12 PM, Nikolay Sivov wrote:
@@ -209,6 +210,7 @@ typedef struct _xmldoc_priv { LONG refs; struct list orphans; domdoc_properties* properties;
- BOOL locked; } xmldoc_priv;
Why does it have to be in _priv?
I put it there so it could be accessed by the Node functions. For example so node_append_child can tell whether the document is locked.
diff --git a/dlls/msxml3/node.c b/dlls/msxml3/node.c index 67f322eb0e5..9cbb2305830 100644 --- a/dlls/msxml3/node.c +++ b/dlls/msxml3/node.c @@ -457,6 +457,8 @@ HRESULT node_insert_before(xmlnode *This, IXMLDOMNode *new_child, const VARIANT if(!new_child) return E_INVALIDARG;
- if (xmldoc_get_locked(This->node->doc)) return E_FAIL;
node_obj = get_node_obj(new_child); if(!node_obj) return E_FAIL;@@ -569,6 +571,8 @@ HRESULT node_replace_child(xmlnode *This, IXMLDOMNode *newChild, IXMLDOMNode *ol if(!newChild || !oldChild) return E_INVALIDARG;
- if (xmldoc_get_locked(This->node->doc)) return E_FAIL;
if(ret) *ret = NULL;@@ -627,6 +631,8 @@ HRESULT node_remove_child(xmlnode *This, IXMLDOMNode* child, IXMLDOMNode** oldCh
if(!child) return E_INVALIDARG;
- if (xmldoc_get_locked(This->node->doc)) return E_FAIL;
if(oldChild) *oldChild = NULL;Maybe, but there's many more methods that modify the tree.
Yes, in fact I missed some. I could write a test that sets domdoc output, then calls all of them - to figure out which ones would need to have this check.
+static HRESULT writer_end_tag_domdoc(mxwriter *writer, const WCHAR *qname, int len) {
- HRESULT hr;
- IXMLDOMNode *parent_node;
- IWineXMLDOMDocumentLock_put_locked(writer->doc_lock, 0);
- hr = domdoc_maybe_write_chars(writer);
- if (FAILED(hr)) return hr;
- IWineXMLDOMDocumentLock_put_locked(writer->doc_lock, 1);
- hr = IXMLDOMNode_get_parentNode(writer->current_node, &parent_node);
- if (FAILED(hr)) return hr;
- IXMLDOMNode_Release(writer->current_node);
- writer->current_node = parent_node;
- return hr;
+}
I don't understand this pattern. Idea I had was to lock when output is set to a document, and unlock when everything is written. Is that wrong?
The mxwriter needs to somehow be able to write to the domdoc even when the domdoc is locked. So it either needs a private interface that skips the locked check, or for the lock to be lifted temporarily.
FWIW I don't like this because it isn't thread safe. If there were some badly-behaved code writing to a domdoc in one thread while an mxwriter was writing in another thread, there could be some spurious writes to the domdoc.
I think the mxwriter's private interface should include functions that skip the locked check. It will also need IXMLDOMNodeLock (in addition to IXMLDOMDocumentLock) in order to call appendChild during startElement and due to characters.
+static HRESULT writer_start_tag_string_len_stream(mxwriter *writer, const WCHAR *qname, int len) +{
- static const WCHAR ltW[] = {'<'};
- close_element_starttag(writer);
- set_element_name(writer, qname ? qname : emptyW, qname ? len : 0);
- write_node_indent(writer);
- write_output_buffer(writer, ltW, 1);
- write_output_buffer(writer, qname ? qname : emptyW, qname ? len : 0);
- writer_inc_indent(writer);
- return S_OK;
+}
+static HRESULT writer_start_tag_bstr_stream(mxwriter *writer, BSTR name) +{
- return writer_start_tag_string_len_stream(writer, name, SysStringLen(name));
+}
Do you need both?
Added both because the SAX and VBSAX handlers use WCHAR and BSTR respectively - and likewise the stream destination operates on WCHARs and the domdoc destination operates on BSTRs. So having both and having one call into the other (bstr { string_len(); } for stream, string_len { bstr(); } for domdoc) avoids converting back and forth in some cases.
Thanks for your comments, Jefferson