> The way you are checking for Win9x is discouraged. We
> rather not have GetVersion() but instead rely on behaviour
> to check for Win9x.
The checks I've inserted for Win9x are strictly to prevent crashing the OS.
There were some previous checks in there and since I've had to add more checks,
I made an inline function for it. Prior to NT, ms wasn't checking validity of
handles in many cases, so calls to SetCursor and DestroyCursor with known
invalid handles are what I believe is bringing them down. All of those should
now be disabled on win9x. I wouldn't normally focus on such a thing so
intensively (passing invalid handles), but this is precisely the type of issue
that's been causing hangs and other annoyances in Lord of the Rings Online
since their upgrade late last year, which started me working on these fixes.
> test_DestroyCursorIcon returns a BOOL but most of the calls
> don't use the returned value. A few do but then the return
> value is not checked against, for example:
> 996 /* Clean up. */
> 997 ret =
> test_DestroyCursorIcon(handle, TRUE, TRUE, __LINE__,
> 0xdeadbeef);
Yea, when I added test_DestroyCursorIcon, I replaced all of the calls to
DestroyCursor and DestroyIcon that didn't have special checks like ok(!ret,
broken(ret), "..."), but I wanted the return value to be available in case it
was needed later in the test code. I've removed all of the cases where the
return value was captured in a variable and then never used and now nothing
uses the return value, but I figure it's probably better to leave it returning
the value than remove it in case some test modification later needs it for
something.
> There are also some spelling fixes needed (minor though,
> I'd guess):
hehe, lots of spelling errors, thanks!
> 22 * FIXME: Add tests for CreateCursor and verify
> that width & hight cannot exceed
> Btw. This FIXME doesn't look right.
The problem is specifically with the CreateCursor function. When I analysed
it, I couldn't find any place that max height & width were checked and the
GetSystemMetrics calls for those values are just hard-coded to return 32.
Personally, I don't understand the X Windows implications well enough to delve
further at this point, but I know that we are definitely missing tests for
CreateCursor as well as the hidden 16-bit API function DestroyIcon32() which
I've modified in my patches (yet to be submitted).
> 78 * shouldFail [I] Rather of not it should
> fail
> Not sure what you are trying to say here.
I clarified this documentation better. Essentially, TRUE if the call to
DestroyCursor or DestroyIcon is expected to fail.
I think I'll have the time so I'm going to try to get a new patch to you today.
Thanks!
Daniel
