[RFC] gameux.dll partial implementation

Mariusz Pluciński vshader at gmail.com
Tue Jul 13 05:18:48 CDT 2010


Hello
First, thank you very much for checking my code. That's good to know
my design is valid and I can continue creating code in this way.
Of course, I know that my code will probably be rejected on the beginning,
but it's still better for me when someone reviews my code, than working
completely without checking all the time.

> I believe that winetest.exe will not start the test on Windows systems
> that are missing gameux.dll, so you shouldn't have to worry about this
> as long as all native versions of gameux.dll contain the class.
I didn't know about that. Thanks.

> I think you can get away with an lstrcmpW here. Yes, technically it
> can behave differently depending on locale, but you're doing a
> case-sensitive compare with a string with only ascii characters. Also,
> everyone else uses lstrcmpW.
You're right, I'll replace it.

> +IMPORTS   = ole32 user32 kernel32 advapi32 oleaut32 msvcrt shell32
>
> The msvcrt import looks suspicious to me, but I can't say how you
> should avoid using the wtof function later.

> +                    GDFData->fPerformanceRatingMinimum =
> _wtof((LPCWSTR)bstrValue);
>
> Maybe you could avoid converting this to a float, or perhaps convert
> to ANSI first? Then you wouldn't have to import msvcrt.
Why should I avoid using msvcrt, when e.g. Microsoft's implementation
of gameux.dll uses it?

> I think you should avoid internal stub functions. Instead, you should
> initially leave out the code that uses your internal function, then
> add a partially implemented implemented internal function and code
> that uses it in the same patch.
If not code freeze,  I would know about it after sending first patch with
internal stub. Now I need to fix a little more code.

>
> +    LPWSTR values[5];
>
> Can there be more than 5 genres?
>
> Perhaps you should check the length of the IXMLDOMNodeList and
> allocate your array first based on that length?
>
> Same applies to the other node types with unknown sizes.
MSDN says that there can be maximum 5 genres. If there are more than
5 in GDF, I simply skip them. This is why I used array with fixed size.

> hr = DoSomething();
>
> if (SUCCEEDED(hr))
>    hr = DoSomethingElse();
>
> if (SUCCEEDED(hr))
>    hr = DoAnotherThing();
Thanks for great strategy. I also dislike when there's lot of indentations,
but didn't know how to solve it without goto (which in my opinion is even
more unreadable).

> (In fact, all internal
> functions meant to be used only in one C file should be static.)
I must admit, that Wine is first code in my life where I saw wide usage of
static functions in C. This is probably why I often forget about using them.

Again, thanks very much. I hope that code freeze will really finish in this
week, and your advices will increase chances for my code to be accepted.



More information about the wine-devel mailing list