Re: [PATCH 3/6] gameux: add support of loading Application Id into registry

[Nikolay Sivov]

On 9/3/2010 13:25, Mariusz Pluciński wrote:
> +static HRESULT _ParseGameDefinition(
> +        IXMLDOMElement *lpXMLGameDefinitionElement,
> +        struct GAME_DATA *GameData)
> +{
> +    static const WCHAR sGameId[] = {'g','a','m','e','I','D',0};
> +
> +    HRESULT hr = S_OK;
> +    BSTR bstrAttribute = NULL;
> +    VARIANT variant;
> No need for initializers here.
> +    hr = IXMLDOMElement_getAttribute(lpXMLGameDefinitionElement, bstrAttribute,&variant);
> +    if(hr != S_OK || V_VT(&variant) != VT_BSTR) hr = E_FAIL;
> No need to test for hr != S_OK and alter failure code after that I guess.
> +    /* load MSXML parser */
> +    hr = CoCreateInstance(&CLSID_DOMDocument30, NULL, CLSCTX_INPROC_SERVER,
> +&IID_IXMLDOMDocument, (LPVOID*)&lpXMLDocument);
> Use neutral CLSID_DOMDocument. Minor things - useless comment, and 
> LPVOID* -> void**.
> +    IXMLDOMDocument *lpXMLDocument = NULL;
> +    IXMLDOMNode *lpXMLGameDefinitionNode = NULL;
> +    IXMLDOMElement *lpXMLRootElement = NULL, *lpXMLGameDefinitionElement = NULL;
> Same here, you don't need to initialize that. Please use shorter names, 
> like 'root', 'document' or something.
> +            hr = IXMLDOMDocument_get_documentElement(lpXMLDocument,&lpXMLRootElement);
> +            if(hr == S_FALSE)
> +                hr = E_FAIL;
> hr != S_OK works.

P.S. it makes sense to test xml parsing part with native msxml first. 
This will hopefully show some problems with builtin msxml.