March 2014 - Wine-Devel - list.winehq.org

Re: [PATCH] ddraw/tests: Only set DDSCAPS_OVERLAY if supported.

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-03-10 16:01, schrieb Henri Verbeet: > On 10 March 2014 14:46, Stefan Dösinger <stefan(a)codeweavers.com> > wrote: >> + hr = IDirectDraw4_GetCaps(ddraw, &hal_caps, &hel_caps); > You're not using "hel_caps". I believe setting the second parameter to NULL crashes on some Windows versions and/or some ddraw versions. dsurface.c suggests otherwise at least for IDirectDraw though. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJTHeRkAAoJEN0/YqbEcdMwPPgP/A6RPz1PoUlnRLLyWw2gjpNr vKDHG8TDtNHOeIvfRcEmTWvomlvDkiEOTA7AxF2dNMtTUtyJkp+CjVGV85NfjWWh YZj7NND2jprunMamYoRg/zb41hIxZ2SuuUYIO8JiISD6SMNwT8wm0fEArMG3rQBy pAUJIZjy1Ats1BBjofG9hJKA3hEBn/sh9URfqt22KQqnVtkfenAmG9jmNNAvtGTZ ipuoT4POChb6HB7L8AEocvWyPBbko/yYbiHOp9HP8sCXdtVmn+EcfrMG7ZwbZYZA O7xOEtMoQvFGhr4MpE18rlmxIIwMwK1+z0UoVWZoflc5tm+NNTCPLmR+jGWtiQKH +5r4fQGh+eQquEpm+RCaD/flz2A6Y+/mE1vdvPlzpZSSkim9RjELQRXjrWO4Df1i K2TZbyS6yY+lZF9icS7BDt8NnKqR71pw/9HYfyWD66BvpXskIj4ACIFxTErAvcVV cj7n0NlRKsLXRspIvYiS4rsVSyh8G0eYwLHZeqffiKLXxWuZ/Q1SWxdZlklXPyfU /0RWNl4HnNRlNDZBIc11ErkQPflghpSYzm4qRD1rFL+lQ6xNnccBJ5HA4/p7C2k9 4pURmRy/IqKx8pEbks9lwaiMFuUlFVffQ2pdSja33NRGk8X+EbRJmxI9oDRiEc2T xUhRPBQyWrCAjewDJZMy =P89h -----END PGP SIGNATURE-----

March 10, 2014

Re: [PATCH 1/2] scrrun: Fix Next() for file collection

by Francois Gouget

On Mon, 10 Mar 2014, Nikolay Sivov wrote: [...] > > It usually sends results but had trouble in the last two runs. > > Here's its last full report (no scrun failures there): > > http://test.winehq.org/data/49f3b4282d29890fe3c213d096571af4de2c45cd/index_… > > > > http://test.winehq.org/data/tests/scrrun:filesystem.html > > > I manually tried it today with and without a patch I sent, and without this > patch it did fail for me with permissions denied error which is not expect > failure reason in this case. It could depend on what current dir is during > manual run and automated run. But anyway I want this patch, cause it makes > sure that we're using valid temp location. Sure, I just meant that the VM used to and should be producing test results again. Your mention of the current directory is interesting as this is the only WineTestBot VM that runs the tests from c:\. On that Windows 2000 that's a writable directory though (at least for the administrator). -- Francois Gouget <fgouget(a)codeweavers.com>

March 10, 2014

Re: Wine Test Bot 1.7.14 analysis

by Jeremy White

> I'll see about tweaking that (I changed it to prevent '-FFFFFFFFF' from > being considered 'fixed' <grin>. But I can fix that a different way). I've updated it. http://www.winehq.org/~jwhite/ecd24b5a874e.html We now have 10 purported fixes! Woohoo! I've also added annotations, which was the main feature I was planning, so I could work the list more intelligently. I've hopefully added in the relevant notes from Francois. My main focus is on the test bot vms, as those match the automated patch screen. (Doesn't mean we shouldn't fix some of the other interesting tests; I'm just trying to limit the scope). Cheers, Jeremy

March 10, 2014

Re: [RFC 2/11] Linux FF: Linux effect status management

by Andrew Eikum

On Fri, Mar 07, 2014 at 10:23:18PM +0100, Elias Vanderstuyft wrote: > On Wed, Mar 5, 2014 at 3:54 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > > On Tue, Mar 04, 2014 at 08:19:39PM +0100, Elias Vanderstuyft wrote: > >> => This has been discussed (see > >> http://www.mail-archive.com/linux-input(a)vger.kernel.org/msg08513.html) > >> and the following is true: > >> My opinion appeared to be correct, but for to be sure, I was > >> recommended to apply the change in Wine as well. > >> > > > > Yes, we should also fix this in Wine to support "buggy" old kernels. > > It would be worth adding a short comment to the code explaining which > > kernel versions are affected by this bug. > > At the moment, I did not receive any reaction yet on my kernel patch > (although it was suggested by one of the kernel devs): > http://www.spinics.net/lists/linux-input/msg29920.html > So I don't know yet in what kernel the bug will be resolved. > If I my Wine patches make it first to release, what should I write > then? "pending as of <current date>"? > Oh, I didn't know it wasn't fixed yet. Maybe just briefly mention that the parameter can be incorrectly modified by the kernel. The Git commit date will point future developers to the correct timeframe. > >> b) Be more precise in returning errors. > >> > > > > Seems fine. If you can add tests (EINVAL seems easy to test, at > > least), that would be even better. > > But that may be FF driver dependent, for example: > - the (soon deprecated) ff-memless driver does not return an error > when setting periodic effects' period to zero > - the newer ff-memless-next driver does return an error when > setting periodic effects' period to zero, and when setting other > invalid parameters > And I don't know whether Wine tests may rely on drivers? (normally, > then you would also need to own a FF capable joystick) > Unless you use a dummy-device, of course, but again, I don't think we > want to include that in the Wine test tree, right? > > However on the other hand, I do understand that this should be tested. > What do you suggest? > To be clear, I meant testing dinput's behavior on Windows. If this behavior changes on Windows based on which device you're using, then you're correct, we can't really write sane tests for it. > >> c) The following in dinput/effect_linuxinput.c:336 : > >> if (res != DI_OK) > >> should be probably : > >> if (FAILED(res)) > >> for example if a device reports S_FALSE because it has already > >> updated an identical effect. > >> > >> The same for line 549: > >> if (retval != DI_OK) > >> should be then : > >> if (FAILED(retval)) > >> > > > > Sure, seems fine. As an additional fix, you could demonstrate with > > tests that Download and Start can return S_FALSE under those > > circumstances on Windows, and fix the Wine behavior to match. > > I'm new to Wine's test-framework, suppose I managed to create such > test and successfully run it on Wine, how can I compile that test for > Windows? (I don't have MS VS, and I prefer to not use it) There's instructions for cross compiling Wine tests here: http://wiki.winehq.org/CompilingDLLsUsingMingw In short, after installing the correct cross-compiler, just run "make crosstest" in the DLL's tests directory. > (Maybe I should also mention that I don't have enough time for the > moment to create tests.) > Many of your patches don't need tests, as they're obviously correct or are making corrections to Linux-specific behavior. There are some patches where you need tests to determine correct Windows behavior. You could create a bug in Bugzilla and attach your patches there, mentioning what tests need to be written. Then the patches won't be lost on the mailing list, and perhaps another developer could take the time to write the tests confirming your changes. Andrew

March 10, 2014

Re: [RFC 1/11] Linux FF: Joystick control panel program

by Andrew Eikum

On Fri, Mar 07, 2014 at 10:01:30PM +0100, Elias Vanderstuyft wrote: > On Wed, Mar 5, 2014 at 3:49 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > > On Tue, Mar 04, 2014 at 08:19:16PM +0100, Elias Vanderstuyft wrote: > >> a) When unloading conditional effects in joy.cpl, the program crashes: > >> When unloading the conditional effect array entries, > >> Wine tries to access an invalid (high) address because > >> dieffect.lpvTypeSpecificParams was not set at the time the > >> effect was created. > >> So declare a valid 'standard' (maxed out) conditional effect. > >> For other unknown effects: > >> - The same can happen when CustomForce effects are available, > >> but because Wine's FF dinput->linux translation does > >> not support this yet, > >> we simply print a FIXME to indicate a possible cause > >> if a crash would happen. > >> - The same for other (unsupported) effects (this is > >> probably not safe?), > >> but then we print a WARN. > > > > For unhandled types, it's probably best to print a FIXME and return > > DIENUM_CONTINUE, rather than continue to rely on uninitialized data. > > Alright. > Should I also write the extra if-test for the CustomForce type, or do > I have to cover it with one if-test as an unhandled type? > I'd just treat it as an unhandled type. > > Additionally, our IDirectInputEffect implementations could check for > > NULL lpvTypeSpecificParams and fail appropriately instead of crashing, > > if this is what Windows does. > > Well, I checked the SetParameters() function, and it does correctly > check on NULL lpvTypeSpecificParams and fail appropriately. > The problem seems again to be related with joy.cpl/main.c: there is no > check performed on the result of IDirectInputDevice2_CreateEffect(), > so change joy.cpl/main.c:784 : > hr = IDirectInputDevice2_CreateEffect( > joystick->device, &pdei->guid, &dieffect, > &joystick->effects[joystick->cur_effect].effect, NULL); > > joystick->effects[joystick->cur_effect].params = dieffect; > joystick->effects[joystick->cur_effect].info = *pdei; > joystick->cur_effect += 1; > > return DIENUM_CONTINUE; > to : > hr = IDirectInputDevice2_CreateEffect( > joystick->device, &pdei->guid, &dieffect, > &joystick->effects[joystick->cur_effect].effect, NULL); > if (FAILED(hr)) > return DIENUM_CONTINUE; > > joystick->effects[joystick->cur_effect].params = dieffect; > joystick->effects[joystick->cur_effect].info = *pdei; > joystick->cur_effect += 1; > > return DIENUM_CONTINUE; > Sure. > Before I knew the joy.cpl app was to be blamed, and was searching in > the IDirectInputEffect implementions, I saw something that is not > conform MSDN: > JoystickAImpl_EnumEffects and JoystickWImpl_EnumEffects should check > on DIENUM_CONTINUE to continue the enumeration, or DIENUM_STOP to stop > it: > http://msdn.microsoft.com/en-us/library/windows/desktop/microsoft.directx_s… > For example, in JoystickWImpl_EnumEffects(), each occurrence of > "(*lpCallback)(&dei, pvRef);" should check whether the return value is > DIENUM_CONTINUE or DIENUM_STOP. When the latter is the case, should we > then first add a "goto" statement to jump to the last "if (xfd == -1) > IDirectInputDevice8_Unacquire(iface);" statement, or directly return? > And what should it return in that case, "DIERR_NOTINITIALIZED" (does > stopping enumeration mean failure? that's not clear in MSDN), or > "DI_OK"? Probably the goto. I'm not sure about the return value, you would need tests to be certain. Probably DI_OK is correct. > And even worse, what if the return value of "(*lpCallback)(&dei, > pvRef);" is not equal to DIENUM_CONTINUE or DIENUM_STOP? Should > JoystickWImpl_EnumEffects() interpret it as the former or as the > latter? (also not mentioned in MSDN) > Again, you'd need tests to confirm. > dieffect.cAxes = 2; > dieffect.rgdwAxes = axes; > dieffect.rglDirection = direction; > > DIRAMPFORCE rforce; > DICONSTANTFORCE cforce; > DIPERIODIC pforce; > DICONDITION cxyforce[2]; > > if (IsEqualGUID(&pdei->guid, &GUID_RampForce)) > Yes, but the declarations have to be at the very top of the function, alongside all of the other declarations. Andrew

March 10, 2014

Re: Wine Test Bot 1.7.14 analysis

by Nikolay Sivov

On 3/10/2014 19:10, Francois Gouget wrote: > On Mon, 10 Mar 2014, Nikolay Sivov wrote: > >> On 3/10/2014 06:13, Jeremy White wrote: >> >> Regarding Mac failures, it looks like this problem: >> >> http://test.winehq.org/data/ecd24b5a874ead368c8f6e9d6981bb0e02472f9d/mac_fg… >> >> is about missing/old libxslt > What's interesting is that the macdrv tests do not run into this issue. > > The difference is that for the macdrv tests I set > DYLD_FALLBACK_LIBRARY_PATH="/opt/local/lib" which causes them to use > what I believe to be the MacPorts libxslt.dylib library. > > For the x11drv tests I set DYLD_FALLBACK_LIBRARY_PATH="/opt/X11/lib" > because the libX11.dylib library does not (or did not) play well with > XQuartz. As a result the X11 tests use /usr/lib/libxslt.dylib which is > the library shipped with Snow Leopard. > > I'll retry running the x11drv tests with /opt/local/lib. > > > However given that MacPorts is not part of Mac OS X that raises the > question of what sort of system tweaks make sense in order to run the > Wine conformance tests without error. So it failed while running with system shipped version? I think we should use a version that we expect users to use, and I don't know which one is that. If system version is old enough to cause troubles then we should be using something more up-to-date from MacPorts. System lib is unlikely to be updated with system update, right? Especially for system that are not supported anymore (not sure if Snow Leopard still gets updates). If it's decided to use system lib no matter what then test should be improved to skip in such cases.

March 10, 2014

Re: [PATCH 1/2] scrrun: Fix Next() for file collection

by Nikolay Sivov

On 3/10/2014 19:18, Francois Gouget wrote: > On Mon, 10 Mar 2014, Nikolay Sivov wrote: > [...] >>> === w2000pro (32 bit filesystem) === >>> filesystem.c:140: Test failed: got 0x800a0046 >> This is not related to my patch, it doesn't add new tests or change behaviour >> of existing ones. I think this vm always fails but doesn't submit results so >> we don't see it on test page. > It usually sends results but had trouble in the last two runs. > Here's its last full report (no scrun failures there): > http://test.winehq.org/data/49f3b4282d29890fe3c213d096571af4de2c45cd/index_… > > http://test.winehq.org/data/tests/scrrun:filesystem.html > I manually tried it today with and without a patch I sent, and without this patch it did fail for me with permissions denied error which is not expect failure reason in this case. It could depend on what current dir is during manual run and automated run. But anyway I want this patch, cause it makes sure that we're using valid temp location.

March 10, 2014

Re: [PATCH 1/2] scrrun: Fix Next() for file collection

by Francois Gouget

On Mon, 10 Mar 2014, Nikolay Sivov wrote: [...] > > === w2000pro (32 bit filesystem) === > > filesystem.c:140: Test failed: got 0x800a0046 > This is not related to my patch, it doesn't add new tests or change behaviour > of existing ones. I think this vm always fails but doesn't submit results so > we don't see it on test page. It usually sends results but had trouble in the last two runs. Here's its last full report (no scrun failures there): http://test.winehq.org/data/49f3b4282d29890fe3c213d096571af4de2c45cd/index_… http://test.winehq.org/data/tests/scrrun:filesystem.html -- Francois Gouget <fgouget(a)codeweavers.com>

March 10, 2014

Re: Wine Test Bot 1.7.14 analysis

by Francois Gouget

On Mon, 10 Mar 2014, Nikolay Sivov wrote: > On 3/10/2014 06:13, Jeremy White wrote: > > Regarding Mac failures, it looks like this problem: > > http://test.winehq.org/data/ecd24b5a874ead368c8f6e9d6981bb0e02472f9d/mac_fg… > > is about missing/old libxslt What's interesting is that the macdrv tests do not run into this issue. The difference is that for the macdrv tests I set DYLD_FALLBACK_LIBRARY_PATH="/opt/local/lib" which causes them to use what I believe to be the MacPorts libxslt.dylib library. For the x11drv tests I set DYLD_FALLBACK_LIBRARY_PATH="/opt/X11/lib" because the libX11.dylib library does not (or did not) play well with XQuartz. As a result the X11 tests use /usr/lib/libxslt.dylib which is the library shipped with Snow Leopard. I'll retry running the x11drv tests with /opt/local/lib. However given that MacPorts is not part of Mac OS X that raises the question of what sort of system tweaks make sense in order to run the Wine conformance tests without error. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Broadcast message : fin du monde dans cinq minutes, repentez vous !

March 10, 2014

Re: [PATCH] ddraw/tests: Only set DDSCAPS_OVERLAY if supported.

by Henri Verbeet

On 10 March 2014 14:46, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > + hr = IDirectDraw4_GetCaps(ddraw, &hal_caps, &hel_caps); You're not using "hel_caps". > + if (!overlay_support) > + ddsd.ddsCaps.dwCaps &= ~DDSCAPS_OVERLAY; This will result in trying to create a surface with just DDSCAPS_VIDEOMEMORY set in the caps. What kind of surface do you expect that to create?

March 10, 2014