Try #2: user32: Windows test request (cursors/icons)

Daniel Santos javatroubadour at yahoo.com
Mon Jul 6 14:17:50 CDT 2009


> 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




      




More information about the wine-devel mailing list