From: Nikolay Sivov <nsivov@codeweavers.com>
Subject: Re: [PATCH v3 2/5] d2d1: Partially implement RegisterEffectFromStream().
Message-Id: <d48684fe-28cb-14b0-80a2-bb110b89a58a@codeweavers.com>
Date: Tue, 14 Jun 2022 17:18:47 +0300
In-Reply-To: <20220614040856.1930461-2-zhui@codeweavers.com>
References: <20220614040856.1930461-1-zhui@codeweavers.com> <20220614040856.1930461-2-zhui@codeweavers.com>

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.

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.