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.

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

[Mariusz Pluciński]

W dniu 03.09.2010 13:13, Nikolay Sivov pisze:
> On 9/3/2010 13:25, Mariusz Pluciński wrote:
> > - hr =
> >
> > IXMLDOMDocument_get_documentElement(lpXMLDocument,&lpXMLRootElement);
> > - if(hr == S_FALSE)
> > - hr = E_FAIL;
>
> hr != S_OK works.

I want this function to return E_FAIL in the case there's no root 
element, and other code error when get_documentElement encouraged other 
problem. If I do
> if(hr != S_OK)
>    hr = E_FAIL;

It will also replace other E_* codes with E_FAIL, and this will make 
problems discovering more complicated.
> P.S. it makes sense to test xml parsing part with native msxml first.
> This will hopefully show some problems with builtin msxml.

OK, I'll this about this.