May 2008 - Wine-Devel - list.winehq.org

Re: winex11.drv: Place tablet cursors in slots based on type

by James Hawkins

On Sat, May 31, 2008 at 12:19 PM, John Klehm <xixsimplicityxix(a)gmail.com> wrote: > This patch looks bigger than it really is. A more detailed changelog is this: > No, it's as big as it seems. > - I've added a lot of comments describing how cursor slotting works. > - added code to slot the cursor appropriately from the get go > - use local memory to temporarily store cursor info before slotting it > (this change results in a lot of inserts deletes of '->' to '.' ) > - iterate through cursors with CURSORMAX and ACTIVE instead of gNumCursors > - cursors will always have a type now: PEN, ERASER, or OTHER > - removed cursor slot fixup code. no longer needed > - removed counters used to place the cursor in slot. no longer needed > Each one of those items should be a separate patch. -- James Hawkins

May 31, 2008

Re: dxdiagn: PhysicalMemory parameter is a string not long long

by Vitaliy Margolen

Alexandre Julliard wrote: > Vitaliy Margolen <wine-devel(a)kievinfo.com> writes: > >> Well it really should be "%llu" but Wine doesn't support that. "%lu" >> is used btw at least in one place in the code: >> http://source.winehq.org/source/dlls/ntdll/actctx.c#L1746 > > It's the correct format for a ULONG_PTR. > Oh right missed the part that ULONG_PTR is 64-bit on win64 only. >> What should it be then? The same as wine_dbgstr_longlong? > > I doubt you want it to be in hex. You should use something like > VariantChangeType. > Patch resent. Thanks for suggestion. Vitaliy.

May 31, 2008

Re: [PATCH] WM_IME_SETCONTEXT is sent when a window is activated

by Vitaliy Margolen

Christopher GAUTIER wrote: > Changelog: > * dll/user32/defwnd.c: > WM_IME_SETCONTEXT is sent when a window is activated > > Description: > The WM_IME_SETCONTEXT message should be sent when a window is (de)activated > (source: http://msdn.microsoft.com/en-us/library/ms776117(VS.85).aspx) > The value for lParam was obtained by spying on the messages when > activating and deactivating the window. > Please add a test for this to user32/test/msg.c test Vitaliy.

May 31, 2008

Re: ddraw: Don't release ddraw if a surface is still using it.

by Michael Karcher

Am Samstag, den 31.05.2008, 15:43 +0200 schrieb Jens Albretsen: > > On wine, there is obviously no problem, as wine keeps the DirectDraw > > object itself alive for longer. On Windows, it is not clear to me that > > the DirectDraw object is not destroyed at the IDirectDraw_Release, the > > only thing we know is, that Direct Draw still is working (perhaps > > implemented by a subobject of the DirectDraw object which keeps living). > > Someone please run the test on windows, I have no windows installation. I will do that, no problem. > > I suggest to alternatively write the second part as > > > > + ref = IDirectDraw_Release(lpDD); > > + ok(ref == 0, "Got refcount %ld, expected 0\n", ref); > > + > > + IUnknown_Release(surface); > > It would pretty much kill the idea of the test. OK, I jumped too quickly to my conclusion. In the ddraw log posted to the bug, the reference count really drops down to zero and exactly that object is used afterwards. Counting the QueryInterface and Release calls, I can't understand why the reference count drops to zero. I will investigate further and respond back then. > Anyway it makes wine stop crashing when programs tries to do something crazy > like this. Stefan Dösinger asked me to put a HeapAlloc all memory between the > two Release's to check if the memory of ddraw object would be corrupt in > Windows. It really doesn't matter if programs live by chance in windows, if > we can implement it so it's not by chance anymore. I agree on that point. > Cheers, > Jens Albretsen Regards, Michael Karcher

May 31, 2008

Re: ddraw: Don't release ddraw if a surface is still using it.

by Jens Albretsen

On Saturday 31 May 2008 14:57:25 Michael Karcher wrote: > Am Samstag, den 31.05.2008, 12:08 +0200 schrieb Jens Albretsen: > The patch looks like it would work as advertised, but the following > > snippet in the testcase smells fishy: > > +static unsigned long getRefcount(IUnknown *iface) > > +{ > > + IUnknown_AddRef(iface); > > + return IUnknown_Release(iface); > > +} > > [...] > > > + IDirectDraw_Release(lpDD); > > + > > + ref = getRefcount(lpDD); > > + ok(ref == 0, "Got refcount %ld, expected 0\n", ref); > > + > > + IUnknown_Release(surface); > > The IDirectDraw_Release seems to be dropping the reference count of the > DirectDraw object to zero. This is OK, of course, as you might well drop > the last reference. But this might put the DirectDraw object into a > state where AddRef crashes, as parts of the object might get destroyed > when the reference count drops down to zero. > > On wine, there is obviously no problem, as wine keeps the DirectDraw > object itself alive for longer. On Windows, it is not clear to me that > the DirectDraw object is not destroyed at the IDirectDraw_Release, the > only thing we know is, that Direct Draw still is working (perhaps > implemented by a subobject of the DirectDraw object which keeps living). Someone please run the test on windows, I have no windows installation. > I suggest to alternatively write the second part as > > + ref = IDirectDraw_Release(lpDD); > + ok(ref == 0, "Got refcount %ld, expected 0\n", ref); > + > + IUnknown_Release(surface); It would pretty much kill the idea of the test. The game demo does this, and runs in windows. I'm trying to emulated the behavior of windows, it fails miserably in wine as the ddraw object suddenly disappears. I can only see what the demo tries to do and make a fix for that, since I have no way of checking it in windows myself. Anyway it makes wine stop crashing when programs tries to do something crazy like this. Stefan Dösinger asked me to put a HeapAlloc all memory between the two Release's to check if the memory of ddraw object would be corrupt in Windows. It really doesn't matter if programs live by chance in windows, if we can implement it so it's not by chance anymore. Isn't that better. > Regards, > Michael Karcher Cheers, Jens Albretsen

May 31, 2008

Re: ddraw: Don't release ddraw if a surface is still using it.

by Michael Karcher

Am Samstag, den 31.05.2008, 12:08 +0200 schrieb Jens Albretsen: The patch looks like it would work as advertised, but the following snippet in the testcase smells fishy: > +static unsigned long getRefcount(IUnknown *iface) > +{ > + IUnknown_AddRef(iface); > + return IUnknown_Release(iface); > +} [...] > + IDirectDraw_Release(lpDD); > + > + ref = getRefcount(lpDD); > + ok(ref == 0, "Got refcount %ld, expected 0\n", ref); > + > + IUnknown_Release(surface); The IDirectDraw_Release seems to be dropping the reference count of the DirectDraw object to zero. This is OK, of course, as you might well drop the last reference. But this might put the DirectDraw object into a state where AddRef crashes, as parts of the object might get destroyed when the reference count drops down to zero. On wine, there is obviously no problem, as wine keeps the DirectDraw object itself alive for longer. On Windows, it is not clear to me that the DirectDraw object is not destroyed at the IDirectDraw_Release, the only thing we know is, that Direct Draw still is working (perhaps implemented by a subobject of the DirectDraw object which keeps living). I suggest to alternatively write the second part as + ref = IDirectDraw_Release(lpDD); + ok(ref == 0, "Got refcount %ld, expected 0\n", ref); + + IUnknown_Release(surface); Regards, Michael Karcher

May 31, 2008

Re: Fixing crashes in Tests (OS version check)

by Alistair Leslie-Hughes

Jeff Latimer wrote: > James Hawkins wrote: >> On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes >> <leslie_alistair(a)hotmail.com> wrote: >> >>> Hi, >>> >>> I've being investigating crashes that occur on Win98 when the tests run. I >>> have fixed a couple already which where trivial. When I attempted to fix the >>> snmpapi tests, I have found that i will end up removing alot of tests (~ >>> 17). If I add a check for the windows version, I can skip the tests that >>> will crash, also there is already tests with if(0) around them. What is the >>> policy about checking the version of the OS before running certain tests? >> Version checks are not allowed. > Is this actually a sensible policy? As Windows version age (Win95 > arrived 12 years ago in 96!) there are going to be more and more > incompatibilities with later versions. Windows is not backward > compatible. Apps built for Vista are not guaranteed to run on earlier > versions and the Windows api moves along. The upshot is that Wine tests > must be dumbed down (ie. we accept multiple results (Win95 and above) or > we delete the test). The outcome is that we don't actually know what we > are testing as Wine could be producing results for XP, Vista, W2K but it > tests ok. > > Hi Jeff, I agree with you. We don't really know if a test gave the correct value, just it whether passed or failed, but I can live with that. I want to see a excellent coverage of tests for each feature that wine implements (no matter what win version). But having to dump down the tests to support the lowest common windows isn't going to achieve this. Maybe the policy could be changed to allow version checking if it they is more than one OS that crashes due to single particular test. Best Regards Alistair Leslie-Hughes

May 31, 2008

Re: Fixing crashes in Tests (OS version check)

by Paul Vriens

Alistair Leslie-Hughes wrote: > Paul Vriens wrote: >> >>> >> In this particular case you could make use of this fact: >> >> util.c:42:GetProcAddress(SnmpUtilAsnAnyCpy) failed >> util.c:43:GetProcAddress(SnmpUtilAsnAnyFree) failed >> util.c:44:GetProcAddress(SnmpUtilOctetsCmp) failed >> util.c:45:GetProcAddress(SnmpUtilOctetsCpy) failed >> util.c:46:GetProcAddress(SnmpUtilOctetsFree) failed >> util.c:47:GetProcAddress(SnmpUtilOctetsNCmp) failed >> >> This happens to be true for win95, win95, winme and NT4. >> >> So if those functions are not available set a variable. Then use that >> variable to skip some tests that crash. >> > Hi Paul, > > These functions aren't the issue, since they are worked out at runtime. > > An example of one that fails is > ret = SnmpUtilOidToA(NULL); > Under win98, this function is valid to call, but will crash if you call > it will a NULL parameter. I don't think removing this test a good idea, > since we are setting XP as a base, at this passes. > > Best Regards > Alistair Leslie-Hughes > > > > What I meant is that these non available functions are an indication for NT4 and below. This means you can detect by logic whether you are on NT4 (and below) or above. This logic can be used to skip some tests. -- Cheers, Paul.

May 31, 2008

Re: Fixing crashes in Tests (OS version check)

by Alistair Leslie-Hughes

Paul Vriens wrote: > >> > In this particular case you could make use of this fact: > > util.c:42:GetProcAddress(SnmpUtilAsnAnyCpy) failed > util.c:43:GetProcAddress(SnmpUtilAsnAnyFree) failed > util.c:44:GetProcAddress(SnmpUtilOctetsCmp) failed > util.c:45:GetProcAddress(SnmpUtilOctetsCpy) failed > util.c:46:GetProcAddress(SnmpUtilOctetsFree) failed > util.c:47:GetProcAddress(SnmpUtilOctetsNCmp) failed > > This happens to be true for win95, win95, winme and NT4. > > So if those functions are not available set a variable. Then use that variable > to skip some tests that crash. > Hi Paul, These functions aren't the issue, since they are worked out at runtime. An example of one that fails is ret = SnmpUtilOidToA(NULL); Under win98, this function is valid to call, but will crash if you call it will a NULL parameter. I don't think removing this test a good idea, since we are setting XP as a base, at this passes. Best Regards Alistair Leslie-Hughes

May 31, 2008

Re: Fixing crashes in Tests (OS version check)

by Reece Dunn

2008/5/31 Jeff Latimer <lats(a)yless4u.com.au>: > James Hawkins wrote: >> On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes >> <leslie_alistair(a)hotmail.com> wrote: >> >>> Hi, >>> >>> I've being investigating crashes that occur on Win98 when the tests run. I >>> have fixed a couple already which where trivial. When I attempted to fix the >>> snmpapi tests, I have found that i will end up removing alot of tests (~ >>> 17). If I add a check for the windows version, I can skip the tests that >>> will crash, also there is already tests with if(0) around them. What is the >>> policy about checking the version of the OS before running certain tests? >> Version checks are not allowed. > Is this actually a sensible policy? As Windows version age (Win95 > arrived 12 years ago in 96!) there are going to be more and more > incompatibilities with later versions. Windows is not backward > compatible. Apps built for Vista are not guaranteed to run on earlier > versions and the Windows api moves along. The upshot is that Wine tests > must be dumbed down (ie. we accept multiple results (Win95 and above) or > we delete the test). The outcome is that we don't actually know what we > are testing as Wine could be producing results for XP, Vista, W2K but it > tests ok. The reason applications built for Vista are not guaranteed to work on earlier versions is that they depend on new API available in Vista. For example, they may use the new 'glass' API directly, rely on the new menu button support in the button control, or some new behaviour in an existing API. The only case that may cause issues here in the tests is the last one, as Wine may be validly conforming to XP or earlier behaviour. Note though that most of the differences are in what error codes are returned. Applications in Windows should not depend on specific error codes being returned (as they may vary from version to version as the Wine tests verify), just whether the call succeeded or failed, unless those error codes are specifically called out in the documentation - QueryInterface returning E_NOINTERFACE if an interface is not supported by the COM object, for instance. Also with the error codes, they may change with each hot fix, service pack and application (e.g. Internet Explorer) update. For example, the shlwapi istream tests have one case that returns E_NOTIMPL on Windows 2000 except when IE6 or later is installed (i.e. the shlwapi version has been updated to match the one on XP or later). Older versions of Windows don't expose certain API. In this case (as is such in this instance), the tests should not crash but should be skipped with a message saying that the function(s) are not available. This scales better, since the API may not be available on a range of Windows versions, or the API may be added when installing IE6 or IE7. New window and user/common control messages can be handled in a similar way, although I don't know the exact mechanics of how this would work. The interesting case is were you have specific behavioural differences (e.g. crash when passed a NULL argument vs returning an error code such as E_POINTER). In these cases, Wine is free to choose what it does as Windows is not consistent with itself (see the application compatibility issues with Vista). Here, if a specific application is dependant on certain behaviour, Wine should support that and have that documented in the tests. Writing tests that work on all Windows platforms is hard. That does not mean that the tests should use OS checks and I think the existing methodology is the right approach to writing the tests. There will be cases where things can't be tested due to incompatible differences, and that would be an area that could be investigated to see if those can be improved (such as the new buggy functionality and possibly others in that family) to allow support for older documented behaviour in the tests, while keeping the Wine implementation sane. - Reece

May 31, 2008