On 6/14/22 10:18 PM, Nikolay Sivov wrote:
On 6/14/22 07:08, Ziqing Hui wrote:
+ /* Loop inside effect node */ + while (next_xml_node(xml_reader, &node_type, &node_name) == S_OK) + { + if (node_type == XmlNodeType_Element) + { + if (!wcscmp(node_name, L"Property")) + hr = parse_property(xml_reader, reg); + else if (!wcscmp(node_name, L"Inputs")) + hr = parse_inputs(xml_reader, reg);
+ if (FAILED(hr)) + { + IXmlReader_Release(xml_reader); + return hr; + } + } + else if (node_type == XmlNodeType_EndElement) + { + IXmlReader_Release(xml_reader); + return S_OK; + } + }
This should properly skip EndElement of Property and Inputs, so explicit check for EndElement here belongs to "Effect" and not some nested node. Easier way would be to error out on unrecognized element names, and like I said before, returning from parse_property or parse_inputs should already skip nested EndElement.
Sorry for my poor understanding... I'm kind of confused by what "skip" means here. So let us make it more clearly.
I use a while loop like this to iterate through XML elements:
while (next_xml_node(xml_reader, &node_type, &node_name) == S_OK) { ... }
This loop exists in parse_effect_xml, parse_property and parse_inputs.
Does "skip" means something like this? :
while (next_xml_node(xml_reader, &node_type, &node_name) == S_OK) { ... if (node_type == EndElement) return break; }
Does "skip" means that we should not check whether last node type is EndElement after loop is end. (And do that check in parent level?)
Regarding error handling, you might as well create xml_reader at the caller of this helper, so you don't have to release it all the time.
Note that return type of next_xml_node() is incorrect.
This patch also has some stray file attached to it.
@@ -1,6 +1,6 @@ MODULE = d2d1.dll IMPORTLIB = d2d1 -IMPORTS = d3d10_1 dxguid uuid gdi32 user32 advapi32 +IMPORTS = d3d10_1 dxguid uuid gdi32 user32 advapi32 ole32 xmllite DELAYIMPORTS = dwrite
New imports might as well go to DELAYIMPORTS, I don't think we necessarily will have to go through xml parsing for builtin effects.