- wine-devel - winehq.org

Re: [PATCH] winejoystick.drv: Revert 62876f3fc5feb1ca23d27a6de5adca580d963d0c
by Sebastian Lackner 05 Sep '16

05 Sep '16

On 03.09.2016 22:21, Bruno Jesus wrote: > Copy & paste fail on the commit ID in previous email, sorry. > > With Aric's intensive steps into commiting HID code it is simpler to revert this patch (which caused a regression) instead of trying alterantive ways to fix it. Specially because HID will change this whole file anyway. > > Superseeds 126184. > > Fixes https://bugs.winehq.org/show_bug.cgi?id=41217 > > Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com> > --- > dlls/winejoystick.drv/joystick_linux.c | 35 ++++++++-------------------------- > 1 file changed, 8 insertions(+), 27 deletions(-) I am not sure if it makes much sense to revert your improvements, just because of the ongoing HID work. When I understand the problem correctly, the performance issues can also be fixed without in a different way. I think your previous patch was already in the right direction, my only criticm was the use of malloc/free instead of WINAPI. Regards, Sebastian

3 3

HLSL compiler
by Fabian Maurer 05 Sep '16

05 Sep '16

I'd like to take a try at Directx10 support, firstmost hacking the HLSL compiler (namely implementing "fx_4_0"). No guarantee I'll succeed though. First, is there already someone working on this? Don't wanna duplicate the effort again. Second, since there already is an existing parser: Did you plan to simply extend it and make one parser for all HLSL versions, or to duplicate the code and have it separate for each version like the official compiler did it? I think I'd just add it to d3dcompiler_43.

3 7

Re: dsound: Avoid using the COM method implementations directly.
by Andrew Eikum 05 Sep '16

05 Sep '16

I guess I'm not opposed to this, but are these changes useful? Andrew On Fri, Sep 02, 2016 at 10:37:33AM +0200, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > dlls/dsound/capture.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/dsound/capture.c b/dlls/dsound/capture.c > index 30870ae..38e3045 100644 > --- a/dlls/dsound/capture.c > +++ b/dlls/dsound/capture.c > @@ -861,7 +861,7 @@ static ULONG DirectSoundCaptureDevice_Release( > LeaveCriticalSection(&DSOUND_capturers_lock); > > if (device->capture_buffer) > - IDirectSoundCaptureBufferImpl_Release(&device->capture_buffer->IDirectSoundCaptureBuffer8_iface); > + IDirectSoundCaptureBuffer8_Release(&device->capture_buffer->IDirectSoundCaptureBuffer8_iface); > > if(device->mmdevice) > IMMDevice_Release(device->mmdevice); > -- > 2.7.4 > >

3 6

Re: [PATCH 1/2] user32/tests: ValidateRect(NULL, &rect) on >= Win 8 no longer invalidates all windows.
by Marvin 05 Sep '16

05 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25559 Your paranoid android. === wvistau64 (32 bit msg) === msg.c:11043: Test failed: expected PeekMessage to return FALSE, got 1 === w864 (32 bit msg) === msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:13933: Test failed: WM_NCRBUTTONDOWN on caption: 3: the msg sequence is not complete: expected 007b - actual 0000 === w1064 (32 bit msg) === msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:7438: Test failed: destroy child on thread exit: 3: the msg sequence is not complete: expected 0000 - actual 001a msg.c:13933: Test failed: WM_NCRBUTTONDOWN on caption: 3: the msg sequence is not complete: expected 007b - actual 0000 === w864 (64 bit msg) === msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:13933: Test failed: WM_NCRBUTTONDOWN on caption: 3: the msg sequence is not complete: expected 007b - actual 0000 === w1064 (64 bit msg) === msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:6488: Test failed: Regions are different msg.c:7438: Test failed: destroy child on thread exit: 3: the msg sequence is not complete: expected 0000 - actual 001a msg.c:13933: Test failed: WM_NCRBUTTONDOWN on caption: 3: the msg sequence is not complete: expected 007b - actual 0000

1 0

Re: [PATCH] msi: avoid potential overflow
by Hans Leidekker 05 Sep '16

05 Sep '16

On Sat, 2016-09-03 at 22:55 +0200, Marcus Meissner wrote: > the create_test_guid function uses MAX_PATH characters > on both passed strings. While it never fully fills them, just > pass in buffers of that size. I'd prefer to fix create_test_guid instead and update callers that pass MAX_PATH buffers accordingly.

1 0

Re: [PATCH 6/6] ddraw/tests: Add D3DVIEWPORT2 transform tests.
by Marvin 03 Sep '16

03 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25550 Your paranoid android. === w1064 (32 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 4/6] ddraw: Apply legacy clipspace to d3d2.
by Marvin 03 Sep '16

03 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25549 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 3/6] ddraw/tests: Test d3d2 and 3 drawing with non-standard viewports.
by Marvin 03 Sep '16

03 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25548 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 2/6] ddraw: Store d3d1 projection matrices in device->legacy_projection.
by Marvin 03 Sep '16

03 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25547 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 1/6] ddraw/tests: Add position tests for D3DPROCESSVERTICES_TRANSFORM.
by Marvin 03 Sep '16

03 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25546 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Where is my address space?
by Snorri Sturluson 03 Sep '16

03 Sep '16

I’m trying to understand the memory usage of Wine. My focus is to make EVE Online run on the Mac as smoothly as possible, and one of the issues I’m running into is that the game runs out of memory in heavy scenes, especially when running at high resolutions on Macs with Retina screens. On Windows, a scene with 300 ships in space may be running in 2.1GB of page file usage, but the same scene under Wine will show a Virtual Memory size of 3.5GB or more. Once the Virtual Memory size (as shown by the Activity Monitor) gets too close to the 4GB mark I start getting errors from Wine, failing to allocate memory, usually leading to a crash. There is a significant difference even when running at the same resolution on both Windows and Mac, and further pronounced if I run in the full 5k resolution of an iMac. Can anyone explain this big difference in memory use between the Windows and Wine sessions? Or give me ideas on how I can figure it out? Or better yet, have suggestions on how to reduce it? Best regards, Snorri Sturluson CCP Games

4 9

"outsb" Emulation
by Morten Welinder 03 Sep '16

03 Sep '16

I'm sorry, it's all my fault! Let me explain. A while ago -- well, 20+ years ago -- I was reading Wine code over and discovered a number of problems big and small. I have no recollection of what they actually were, but I shipped the list off to Alexandre who took at least some of my suggestions. Including one that was wrong. And that is why we have code like http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/krnl386.exe16/instr.c… That FIXME is old -- older than the bug, actually. The answer is that no, it's not correct, but it used to be. "outsb" allows segment override, even if insb does not, see http://x86.renejeschke.de/html/file_module_x86_id_223.html The wine source file has gone though a few renames since then. I tracked it back to here: http://source.winehq.org/git/wine.git/history/58371aa361c7b119a5880b2f69d84… The first bad version occurs on 960114 -- so long ago that two-digit years were still a thing. I realize that it doesn't matter in practical terms. The code is unlikely to have been run multiple times over the past 20 years. Still, it's my fault. Morten Welinder PS: Please don't take my word for it. I really don't want to return in 2036 with an update. PPS: Keep up the good work!

1 0

Re: [2/2] imm32: Set the default IME window title and styles as in native.
by Alexandre Julliard 02 Sep '16

02 Sep '16

Akihiro Sagawa <sagawa.aki(a)gmail.com> writes: > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/imm32/imm.c | 6 ++++-- > dlls/imm32/tests/imm32.c | 9 +++++++++ > 2 files changed, 13 insertions(+), 2 deletions(-) It fails here: ../../../tools/runtest -q -P wine -T ../../.. -M imm32.dll -p imm32_test.exe.so imm32 && touch imm32.ok imm32.c:888: Test failed: got make: *** [imm32.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

1 0

Implementing Fiber-Local Storage Callbacks: sharing data between DLLs
by John Sheu 02 Sep '16

02 Sep '16

TL;WR: I'd like to share a struct definition between ntdll and kernel32, passed through the PEB. Is this kosher according to Wine style/design, and if so, which header should I stick it in? -- I'd like to have fiber-local storage callbacks in Wine. http://source.winehq.org/git/wine.git/commit/556fef3d http://source.winehq.org/git/wine.git/commit/ab11a5ae http://source.winehq.org/git/wine.git/commit/4e8f43f4 So, after writing the conformance tests, it's time to implement the feature :-) I've got the feature all done, except for one issue. The proximate issue is that when NtTerminateProcess is used on Windows, FLS callbacks are not called for threads that are aborted (and those threads do not get a DLL_THREAD_DETACH notification). However, if during DLL_PROCESS_DETACH the FLS slot is then freed, then the callbacks are called on behalf of those (already-terminated) threads by the process-detaching thread. The behavior appears to be thus: if the thread is exited cleanly (through LdrShutdownThread), then the FLS callbacks are called for that thread; otherwise on an unclean exit the FLS slots for the thread are transferred to some global list, and the callbacks for that list are called when FlsFree is later called. The underlying problem regards doing that transfer to the global list. LdrShutdownThread and its other friends reside in ntdll, so ntdll will have to know how to manage FLS. The FLS API however resides in kernel32, so kernel32 will have to know, as well. For this purpose I'd like to pass a common struct between ntdll and kernel32 through PEB.FlsListHead. What I see though is that include/winternl.h contains (almost all) publicly Microsoft-documented struct definitions, and I don't see anywhere else where such common structs are presently shared between two DLLs. So is it the case that the convention in Wine is to not do such things as sharing structs, or is this also done elsewhere and I just need to find the right header for it? Thanks, -John Sheu

4 6

Re: [3/3] imm32,user32: Create and destroy the default IME window implicitly. (try 9; resend)
by Sebastian Lackner 02 Sep '16

02 Sep '16

On 23.08.2016 15:13, Akihiro Sagawa wrote: > > Try 9: > - Withdraw try 8 changes as we have ImmDisableIME now. > - Rebased and resolve conflicts against ImmDisableIME patch. > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/imm32/imm.c | 99 +++++++++++++++++++++++++++++++++++----------- > dlls/imm32/imm32.spec | 1 + > dlls/imm32/tests/imm32.c | 12 +++--- > dlls/user32/misc.c | 4 +- > dlls/user32/user_private.h | 1 + > dlls/user32/win.c | 26 ++++++++++++ > dlls/user32/win.h | 1 + > 7 files changed, 113 insertions(+), 31 deletions(-) This patch broke various user32 tests in virtual desktop mode, for example: $ (cd dlls/user32/tests/; make broadcast.ok) ../../../tools/runtest -q -P wine -T ../../.. -M user32.dll -p user32_test.exe.so broadcast && touch broadcast.ok broadcast.c:174: Test failed: Asynchronous message sent instead broadcast.c:152: Test failed: Returned: 0 broadcast.c:153: Test failed: Asynchronous message sent instead broadcast.c:174: Test failed: Asynchronous message sent instead broadcast.c:238: Test failed: Returned: 0 broadcast.c:239: Test failed: Asynchronous message sent instead broadcast.c:257: Test failed: Asynchronous message sent instead broadcast.c:238: Test failed: Returned: 0 broadcast.c:239: Test failed: Asynchronous message sent instead broadcast.c:257: Test failed: Asynchronous message sent instead broadcast.c:298: Test failed: Returned: 0 error 1460 broadcast.c:299: Test failed: Asynchronous message sent instead broadcast.c:309: Test failed: Returned: 0 error 1460 broadcast.c:310: Test failed: Asynchronous message sent instead broadcast.c:320: Test failed: Returned: 0 error 1460 broadcast.c:321: Test failed: Asynchronous message sent instead broadcast.c:332: Test failed: Asynchronous message sent instead make: *** [Makefile:197: broadcast.ok] Fehler 17 Could you please take a look?

2 4

Re: [PATCH 2/2] ole32/tests: Update tests to the new channel hook info structure.
by Huw Davies 02 Sep '16

02 Sep '16

On 2 Sep 2016, at 11:32, Huw Davies <huw(a)codeweavers.com> wrote: > > Also add tests for method number and causality. Please ignore this, I’ve sent a better version that doesn’t trace() garbage when the new structs are in use. Huw.

1 0

Re: [PATCH] winejoystick.drv: Fix long time consuming task of looking for joysticks
by Sebastian Lackner 02 Sep '16

02 Sep '16

On 01.09.2016 05:18, Bruno Jesus wrote: > + joysticks = scandir(p, &files, filter_js, sort_js); Usually malloc-like allocations should be avoided in the Wine source. I am not sure if Alexandre is willing to make an exception in this case because its only a very small amount of memory - nevertheless, replacing with a different directory enumeration mechanism should also not be too difficult. Regards, Sebastian

2 1

Re: user32/tests: Mark a test failure with todo_wine.
by Alexandre Julliard 02 Sep '16

02 Sep '16

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > I am getting very consistently test failures here: > > clipboard.c:199: Test failed: 0002: seqno unchanged > clipboard.c:199: Test failed: 0082: seqno unchanged > > Also, currently the clipboard tests trigger all kind of other race-conditions > and bugs in winex11. Are you already aware of those issues, Alexandre? Yes, the stricter tests reveal all sorts of issues with the X11 implementation. I'll get to that once the user32 side works correctly. I don't get the above failure though (not that it means much, sequence number support is also fairly broken). -- Alexandre Julliard julliard(a)winehq.org

1 0

Maximum OpenGL version
by Fabian Maurer 02 Sep '16

02 Sep '16

Currently looking at the directx11 implementation, and I noticed I first need to set a "MaxVersionGL" in the registry to enable creation of 4.3 context and so DX11 support. What's the reason behind this? The change is from a1e718ccabc7c330dd16c7face78022965b03e20 (wined3d: Add a setting for the maximum OpenGL version to use). Why do we need to set the maximum version instead of just using the highest context we can get (and still use) ? Fabian Maurer

3 6

Re: [PATCH 3/3] ddraw/tests: Port test_transform_vertices to ddraw2 and ddraw4.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25522 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw2) === ddraw2.c:2391: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2546: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 2/3] ddraw/tests: Test matrices in TransformVertices.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25521 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 1/3] ddraw/tests: Make the vec parameter of compare_vec4 const.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25520 Your paranoid android. === w1064 (32 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw4) === ddraw4.c:2542: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (32 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw7) === ddraw7.c:2272: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: Add a simple 3D APIs demo
by Matteo Bruni 01 Sep '16

01 Sep '16

2016-08-19 21:41 GMT+02:00 Ruslan Kabatsayev <b7.10110111(a)gmail.com>: > This adds a 3d-demo wine application. It will allow users to check that their > system can handle at least basic WGL, D3D8 and D3D9 before they need to look > deeper into their problem. > > Signed-off-by: Ruslan Kabatsayev <b7.10110111(a)gmail.com> > --- > configure | 26 +- > configure.ac | 1 + > programs/3d-demo/Makefile.in | 14 + > programs/3d-demo/common.c | 107 ++++++ > programs/3d-demo/common.h | 36 ++ > programs/3d-demo/d3d.c | 605 ++++++++++++++++++++++++++++++++ > programs/3d-demo/d3d8.c | 2 + > programs/3d-demo/d3d9.c | 2 + > programs/3d-demo/demo.rc | 61 ++++ > programs/3d-demo/main.c | 175 +++++++++ > programs/3d-demo/resource.h | 33 ++ > programs/3d-demo/wgl.c | 473 +++++++++++++++++++++++++ > programs/3d-demo/winehq_logo_glass.png | Bin 0 -> 31512 bytes > 13 files changed, 1517 insertions(+), 18 deletions(-) > create mode 100644 programs/3d-demo/Makefile.in > create mode 100644 programs/3d-demo/common.c > create mode 100644 programs/3d-demo/common.h > create mode 100644 programs/3d-demo/d3d.c > create mode 100644 programs/3d-demo/d3d8.c > create mode 100644 programs/3d-demo/d3d9.c > create mode 100644 programs/3d-demo/demo.rc > create mode 100644 programs/3d-demo/main.c > create mode 100644 programs/3d-demo/resource.h > create mode 100644 programs/3d-demo/wgl.c > create mode 100644 programs/3d-demo/winehq_logo_glass.png I have a few pretty generic suggestions. You don't need to include configure changes in the patch, they will be regenerated by Alexandre anyway since those depend on the version of the autotools. Did you look into adding those tests to dxdiag instead of as a separate program? I don't think a separate program is necessarily bad but if integrating into dxdiag is reasonable it's probably better to do that from the start. Avoid camelCase or similar mixed-case identifier styles, just use lowercase_with_underscores. For pointer declarations you want to stick the '*' to the variable, not the type. Also please avoid LPTYPES, just use explicit pointer types. C++ comments (// comment) aren't allowed in Wine sources. Please replace them with normal C comments (/* comment */), or drop them entirely if they don't add anything that isn't clear from just looking at the code. It would be better to use explicit float constants when assigning to float variables (e.g. D3DMATERIAL). There aren't many other general style rules in Wine (it's mostly about using the style already used in the component so for new components you're mostly free to pick your own) as long as there is consistency. I see you mixing space after ',' with no space, pick one (preferably the space variant IMO) and stick to it. We usually use parentheses with the "sizeof" operator in Wine sources, so while it's not strictly required it would probably be nice to follow the general trend. More specific comments: I'd split this over multiple patches. You can probably make an initial patch adding the program with no real functionality, then multiple patches adding the various graphical API demos. In that regard, the d3d8 / d3d9 "merged" handling via macros seems a bit obfuscating to me. Just replicating the code twice seems cleaner. Maybe you can create a few helper functions for the common code. BTW, you can probably make things a bit more structured by moving the main loop to common code and have it call API backend functions. You'd have for each backend a structure like: static const struct render_backend d3d9_backend { d3d9_list_modes, d3d9_init, d3d9_draw, d3d9_wndproc, d3d9_destroy, }; (just an idea WRT the actual functions, I haven't really looked in detail) and you'd pass the structure from the selected API to the "main loop" function. I don't think you're using GLU anymore (good) so you should drop the glu32 import. I haven't looked in detail at the actual demos / renderers but I think there is enough stuff to work on already :)

4 6

Re: [v2] Add a GUI for dxdiag program
by Matteo Bruni 01 Sep '16

01 Sep '16

2016-08-27 10:26 GMT+02:00 Ruslan Kabatsayev <b7.10110111(a)gmail.com>: > This adds a dialog with only System tab, reflecting what save-to-file > functionality currently provides. It can also save the report as file > from the GUI in a way similar to its Windows counterpart. > This version now also shows DMI information which current dxdiagn.dll doesn't > provide, but may do so in the future. Also some spacing issues are fixed. > > Signed-off-by: Ruslan Kabatsayev <b7.10110111(a)gmail.com> > --- > programs/dxdiag/Makefile.in | 3 +- > programs/dxdiag/dxdiag.rc | 64 +++++++++++ > programs/dxdiag/dxdiag_private.h | 9 +- > programs/dxdiag/main.c | 20 ++-- > programs/dxdiag/main_dialog.c | 220 ++++++++++++++++++++++++++++++++++++++ > programs/dxdiag/resource.h | 56 ++++++++++ > 6 files changed, 356 insertions(+), 16 deletions(-) > create mode 100644 programs/dxdiag/main_dialog.c > create mode 100644 programs/dxdiag/resource.h Hi Ruslan, first of all, I think this patch should be splitted. For example, you could split the "save all information" button functionality, together with the common dialog include and dll linkage, to a separate patch. Aside from that, I have some additional comments. I'm not exactly a Win32 GUI expert so I might certainly have missed something significant. Anyway... > diff --git a/programs/dxdiag/Makefile.in b/programs/dxdiag/Makefile.in > index 437cc5e..d971a55 100644 > --- a/programs/dxdiag/Makefile.in > +++ b/programs/dxdiag/Makefile.in > @@ -1,10 +1,11 @@ > MODULE = dxdiag.exe > APPMODE = -mwindows -municode > -IMPORTS = dxguid ole32 oleaut32 user32 > +IMPORTS = dxguid ole32 oleaut32 user32 comdlg32 Keep it sorted alphabetically. > > C_SRCS = \ > information.c \ > main.c \ > + main_dialog.c \ > output.c Formatting. > +IDD_SYSTEM_TAB DIALOG 0, 0, 387, 176 > +STYLE DS_SETFONT | WS_CHILDWINDOW > +FONT 8, "Ms Shell Dlg" > +{ > + GROUPBOX "System Information", IDC_SYSINFO_GROUP, 3, 65, 381, 136, 0, WS_EX_LEFT > + LTEXT "If you know what area is causing the problem, click the appropriate tab above. Otherwise, you can use the ""Next Page"" button below to visit each page in sequence.", IDC_SYSTEMTAB_LABEL1, 9, 37, 344, 17, SS_LEFT, WS_EX_LEFT > + LTEXT "This tool reports information about the DirectX components and drivers installed on your system. It lets you test functionality and diagnose problems.", IDC_SYSTEMTAB_LABEL2, 9, 11, 370, 17, SS_LEFT, WS_EX_LEFT This text seems way too similar to the text in the original application. I'm not sure it adds much, maybe just dropping it entirely is fine? > + RTEXT "Current Date/Time:", IDC_SYSTEMTAB_DATETIME_LABEL, 10, 74, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_DATETIME_VALUE, 153, 74, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Computer name:", IDC_SYSTEMTAB_PCNAME_LABEL, 10, 84, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_PCNAME_VALUE, 153, 84, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Operating System:", IDC_SYSTEMTAB_OS_LABEL, 10, 94, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_OS_VALUE, 153, 94, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Language:", IDC_SYSTEMTAB_LANG_LABEL, 10, 104, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_LANG_VALUE, 153, 104, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "System Manufacturer:", IDC_SYSTEMTAB_VENDOR_LABEL, 10, 114, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_VENDOR_VALUE, 153, 114, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "System Model:", IDC_SYSTEMTAB_MODEL_LABEL, 10, 124, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_MODEL_VALUE, 153, 124, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "BIOS:", IDC_SYSTEMTAB_BIOS_LABEL, 10, 134, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_BIOS_VALUE, 153, 134, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Processor:", IDC_SYSTEMTAB_CPU_LABEL, 10, 144, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_CPU_VALUE, 153, 144, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Memory:", IDC_SYSTEMTAB_RAM_LABEL, 10, 154, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_RAM_VALUE, 153, 154, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "Page File:", IDC_SYSTEMTAB_PAGEFILE_LABEL, 10, 164, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_PAGEFILE_VALUE, 153, 164, 227, 9, SS_LEFT, WS_EX_LEFT > + > + RTEXT "DirectX Version:", IDC_SYSTEMTAB_DXVERSION_LABEL, 10, 174, 140, 9, SS_RIGHT, WS_EX_LEFT > + LTEXT "", IDC_SYSTEMTAB_DXVERSION_VALUE, 153, 174, 227, 9, SS_LEFT, WS_EX_LEFT > + > + AUTOCHECKBOX "Check for WHQL digital signatures", IDC_WHQL_CHECKBOX, 10, 186, 369, 9, 0, WS_EX_LEFT > } Similarly here. Not sure about these, I'd drop the unused entries though. > - WINE_FIXME("Information dialog is not implemented\n"); > + DialogBoxW(hInst,(WCHAR*)MAKEINTRESOURCE(IDD_MAIN),NULL,main_dlg_proc); Space after the commas. Also you don't need to cast, just use MAKEINTRESOURCEW() (see for example notepad/dialog.c). > +extern HINSTANCE hInstance; Please avoid camelcase or hungarian notation. > +extern struct dxdiag_information * dxdiag_info; The '*' should be next to the variable name, as in the preexisting dxdiag sources. > +void set_main_dialog_tab(int new_tab_index) > +{ > + RECT tab_rect; > + POINT top_left, bottom_right; > + const HWND new_tab = main_dialog_tabs[new_tab_index]; > + int tab = 0; > + for(;tab<MAIN_DIALOG_TAB_COUNT;++tab) > + ShowWindow(main_dialog_tabs[tab], SW_HIDE); Please leave a blank line between variable declarations and code. Stick to consistently having a whitespace between the '<' and its operands and after the ';'. Also, but this is probably just my preference, I'd initialize tab to 0 in the "for" instead. > +void main_dialog_insert_tab(int index, int caption_id, int template_id, DLGPROC dlg_proc) > +{ > + WCHAR tab_caption[MAX_STRING_LEN]; > + const TCITEMW tab_item = {.pszText = tab_caption, .mask = TCIF_TEXT}; Designated initializers aren't allowed in C89. > + LoadStringW(hInstance, caption_id, tab_caption, sizeof tab_caption/sizeof tab_caption[0]); Parentheses around the sizeof operand. > +void populate_system_tab(void) > +{ > + const HWND page = main_dialog_tabs[SYSTEM_TAB_INDEX]; > + const struct system_information zeros = {0}; > + const struct system_information*const si = dxdiag_info ? &dxdiag_info->system_info : &zeros; Spaces. > +void save_info_dialog(HWND parent) > +{ > + static const WCHAR filter[] = {'T','e','x','t',' ','f','i','l','e',' ','(','*','.','t','x','t',')',0, '*','.','t','x','t',0, > + 'X','M','L',' ','f','i','l','e',' ','(','*','.','x','m','l',')',0, '*','.','x','m','l',0,0}; > + OPENFILENAMEW ofn = {0}; > + WCHAR filename[MAX_PATH]; > + filename[0] = 0; You can initialize filename right in the definition, like: WCHAR filename[MAX_PATH] = {0}; Actually, do you need to initialize it at all? > + ofn.lStructSize = sizeof ofn; > + ofn.hwndOwner = parent; > + ofn.lpstrFilter = filter; > + ofn.lpstrFile = filename; > + ofn.nMaxFile = MAX_PATH; > + ofn.Flags = OFN_EXPLORER|OFN_HIDEREADONLY|OFN_OVERWRITEPROMPT; Spaces. > +void update_dxdiag_info(void) > +{ > + const HWND checkbox = GetDlgItem(main_dialog, IDC_WHQL_CHECKBOX); > + const HWND save_all_button = GetDlgItem(main_dialog, IDC_SAVE_INFO); > + const BOOL check_whql = SendMessageW(checkbox, BM_GETCHECK, 0L, 0L) == BST_CHECKED; > + free_dxdiag_information(dxdiag_info); > + dxdiag_info = collect_dxdiag_information(check_whql); > + /* update the dialog before showing error message */ > + populate_tabs(); > + if(!dxdiag_info) Nitpick, space after "if". > +INT_PTR CALLBACK system_tab_proc(HWND hsystab, UINT msg, WPARAM wparam, LPARAM lparam) > +{ > + switch(msg) > + { > + case WM_COMMAND: > + switch(HIWORD(wparam)) > + { > + case BN_CLICKED: > + switch(wparam) > + { > + case IDC_WHQL_CHECKBOX: > + { > + update_dxdiag_info(); > + return TRUE; > + } > + } This looks pretty ugly. You don't need a separate block inside the case so you can just drop the inner braces. Actually I don't think you're ever going to have other buttons in the system tab so the switch seems unnecessary.

2 3

Re: Bruno Jesus : xinput1_3: Stub XInputGetStateEx.
by Huw Davies 01 Sep '16

01 Sep '16

On 1 Sep 2016, at 15:55, Alexandre Julliard <julliard(a)winehq.org> wrote: > Module: wine > Branch: master > Commit: 0fe758046ddc0f37944b8a3e075b8a5fdc5d81a4 > URL: http://source.winehq.org/git/wine.git/?a=commit;h=0fe758046ddc0f37944b8a3e0… > > Author: Bruno Jesus <00cpxxx(a)gmail.com> > Date: Wed Aug 31 15:17:44 2016 -0300 > > xinput1_3: Stub XInputGetStateEx. > > Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com> > Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> > + if (pXInputGetStateEx_Ordinal) > + ok (pXInputGetStateEx_Ordinal == pXInputGetStateEx, "XInputGetStateEx in the wrong ordinal\n"); > + else > + ok (broken(1), "XInputGetStateEx not found in this dll version\n”) Hi Bruno, This is causing test failures: https://testbot.winehq.org/JobDetails.pl?Key=25476 https://test.winehq.org/data/tests/xinput1_3:xinput.html Could you take a look? Thanks, Huw.

2 1

Experimental gcc optimization for 64-bit wine
by Daniel Santos 01 Sep '16

01 Sep '16

Over the last two weeks I've been working on an experimental gcc optimization for wine that out-of-lines the expensive prologues & epilogues required for an ms x64 abi function to call a 64-bit sysv function. The theory is that the added expense of each function executing a few extra instructions to facilitate this will be offset by the reduction in instruction cache misses. As far as I can tell, gcc's ix86 back end has never supported any type of out-of-line pro/epilogue stubs, so adding this support has been somewhat tedious as it tends to get broken by subsequent optimization passes. It currently works in many cases, but in other cases produces bad code so this is a work in progress. None the less, I thought it was interesting enough to post the current results in terms of .text size reduction. On gcc 5.4.0 (with the 8-byte aligned stack assumption flaw corrected), the total .text size of all dlls on my build was 45.2MiB while the out-of-lined build was 35.9MiB, a reduction of 20.5%, so it looks promising so far. I presume it will be a little while before all of the flaws are worked out so that it always produces good code, but once there I can start to produce some real performance numbers. It's probably also worth mentioning that the ideal long-term solution is to find ways eliminate the register save/restores through various means-- static analysis, better evaluation of branch costs, and other optimizations in Wine its self. For example, there are many WINAPI functions who's only sysv call is to wine_dbg_log() via a TRACE(). Just marking this function as __attribute__((__cold__)) should reduce the number of unneeded register save/restores in the function's main path by a great amount. (I haven't actually tested it yet.) Daniel

2 3

Re: wintrust/tests: Fix test for pfnAddSgnr2Chain.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25506 Your paranoid android. === w864 (32 bit softpub) === softpub: unhandled exception c0000005 at 77723ED2 === w1064 (32 bit softpub) === softpub: unhandled exception c0000005 at 768AC81B === w864 (64 bit softpub) === softpub: unhandled exception c0000005 at 00007FFB7AFF266C === w1064 (64 bit softpub) === softpub.c:254: Test failed: Expected S_OK, got 00000001 softpub.c:257: Test failed: Expected S_OK, got 00000001 softpub: unhandled exception c0000005 at 00007FFC77A79A5D

1 0

Re: rpcrt4/tests: Remove an unnecessary and confusing message.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25500 Your paranoid android. === w864 (32 bit rpc) === rpc.c:812: Test failed: GUID does not appear to contain a MAC address: {f03f0f60-7046-11e6-be78-e71666c2e04a} === w864 (64 bit rpc) === rpc.c:812: Test failed: GUID does not appear to contain a MAC address: {ab19995e-7047-11e6-be78-e71666c2e04a}

1 0

Re: [PATCH] msctf/tests: Return interface pointers instead of pointers to impl structure
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25492 Your paranoid android. === w864 (64 bit inputprocessor) === inputprocessor.c:831: Test failed: Deactivation came unexpectedly

2 1

Re: [PATCH 5/5] ddraw/tests: Extend TransformVertices invalid data tests.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25491 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: ntoskrnl.exe: Stub IoInvalidateDeviceRelations.
by Aric Stewart 01 Sep '16

01 Sep '16

On 9/1/16 2:28 AM, Sebastian Lackner wrote: > From: Aric Stewart <aric(a)codeweavers.com> > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > After we have reduced the size of the implementation a lot, it feels a bit > unnecessary to add a separate pnp_manager.c file. > > @ Aric: Are you fine with this change, or do you have significantly more > PnP work after those initial four patches? > I am find with it being in ntoskrnl.c if that is the decision as to what is best. It does affect the rest of the patch set so if this is where Alexandre agree it should go then that would be useful to get committed so that I can rebase the rest. Also one of my earlier patches (125976) has an implementation of get_device_id in ntoskrnl.c which of course can be replace by the version in this set of patches. -aric > For patch 2 I have some more feedback which I will reply separately. > > dlls/ntoskrnl.exe/ntoskrnl.c | 10 ++++++++++ > dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- > include/ddk/wdm.h | 1 + > 3 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c > index a2cbb74..507ffdd 100644 > --- a/dlls/ntoskrnl.exe/ntoskrnl.c > +++ b/dlls/ntoskrnl.exe/ntoskrnl.c > @@ -4,6 +4,7 @@ > * Copyright (C) 2007 Alexandre Julliard > * Copyright (C) 2010 Damjan Jovanovic > * Copyright (C) 2016 Sebastian Lackner > + * Copyright (C) 2016 CodeWeavers, Aric Stewart > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -2788,3 +2789,12 @@ done: > CloseServiceHandle( service_handle ); > return status; > } > + > + > +/*********************************************************************** > + * IoInvalidateDeviceRelations (NTOSKRNL.EXE.@) > + */ > +void WINAPI IoInvalidateDeviceRelations( DEVICE_OBJECT *device_object, DEVICE_RELATION_TYPE type ) > +{ > + FIXME( "(%p, %i): stub\n", device_object, type ); > +} > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > index 5408e91..1c4d08f 100644 > --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > @@ -400,7 +400,7 @@ > @ stdcall IoInitializeIrp(ptr long long) > @ stdcall IoInitializeRemoveLockEx(ptr long long long long) > @ stdcall IoInitializeTimer(ptr ptr ptr) > -@ stub IoInvalidateDeviceRelations > +@ stdcall IoInvalidateDeviceRelations(ptr long) > @ stub IoInvalidateDeviceState > @ stub IoIsFileOriginRemote > @ stub IoIsOperationSynchronous > diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h > index 68694af..0024968 100644 > --- a/include/ddk/wdm.h > +++ b/include/ddk/wdm.h > @@ -1234,6 +1234,7 @@ PVOID WINAPI IoGetDriverObjectExtension(PDRIVER_OBJECT,PVOID); > PDEVICE_OBJECT WINAPI IoGetRelatedDeviceObject(PFILE_OBJECT); > void WINAPI IoInitializeIrp(IRP*,USHORT,CCHAR); > VOID WINAPI IoInitializeRemoveLockEx(PIO_REMOVE_LOCK,ULONG,ULONG,ULONG,ULONG); > +void WINAPI IoInvalidateDeviceRelations(PDEVICE_OBJECT,DEVICE_RELATION_TYPE); > NTSTATUS WINAPI IoWMIRegistrationControl(PDEVICE_OBJECT,ULONG); > > PKTHREAD WINAPI KeGetCurrentThread(void); >

1 0

Re: [PATCH 3/5] ddraw: Fix offscreen flag handling in TransformVertices.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25490 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). The test timed out

1 0

Re: [PATCH 2/5] ddraw: Copy extra vertex data in viewport::TransformVertices.
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25489 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 1/5] ddraw/tests: Move IDirect3DViewport::TransformVertices tests to ddraw1.c (v2).
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25488 Your paranoid android. === w864 (32 bit d3d) === d3d.c:2051: Test failed: Unexpected destination texture level pixels; 2031 differences at 0 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 518 differences at 1 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 138 differences at 2 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 46 differences at 3 level d3d.c:2235: Test failed: Unexpected destination texture level pixels; 257 differences d3d.c:2382: Test failed: Unexpected destination texture level pixels; 256 differences at 2 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 64 differences at 3 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 16 differences at 4 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 4 differences at 5 level === w1064 (32 bit d3d) === d3d.c:2051: Test failed: Unexpected destination texture level pixels; 2031 differences at 0 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 518 differences at 1 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 138 differences at 2 level d3d.c:2051: Test failed: Unexpected destination texture level pixels; 46 differences at 3 level d3d.c:2235: Test failed: Unexpected destination texture level pixels; 257 differences d3d.c:2382: Test failed: Unexpected destination texture level pixels; 256 differences at 2 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 64 differences at 3 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 16 differences at 4 level d3d.c:2382: Test failed: Unexpected destination texture level pixels; 4 differences at 5 level === w1064 (32 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2240: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 3/4] xinput1_3: Stub XInputGetStateEx
by Marvin 01 Sep '16

01 Sep '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25476 Your paranoid android. === wxppro (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w2003std (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === wvistau64 (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w8 (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w864 (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w1064 (32 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === wvistau64 (64 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w864 (64 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal === w1064 (64 bit xinput) === xinput.c:242: Test failed: XInputGetStateEx in the wrong ordinal

1 0

Re: [PATCH v2 2/4] ntoskrnl.exe: Implement loading plug and play devices
by Sebastian Lackner 01 Sep '16

01 Sep '16

On 31.08.2016 18:38, Aric Stewart wrote: > v2: No longer maintaining a device store > Suggestions from Sebastian Lackner > > We are not handling installing drivers just yet, so we just make use > of the critical device store. These are drivers that, on Windows, are > automatically loaded on boot every time. This database is maintained > in [CurrentControlSet/Control/CriticalDeviceDatabase] The keys in this > registry key represent a HardwareID that match a device. We query the > IDs from the bus device’s BusQueryHardwareIDs and if we find a match > (from most specific to most general) we look at that registry key. The > CriticalDeviceDatabase entry specify a [Service] value representing > the driver. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/pnp_manager.c | 196 +++++++++++++++++++++++++++++++++++++++- > include/ddk/wdm.h | 3 +- > 2 files changed, 197 insertions(+), 2 deletions(-) > > > > v2-0002-ntoskrnl.exe-Implement-loading-plug-and-play-device.txt > > > diff --git a/dlls/ntoskrnl.exe/pnp_manager.c b/dlls/ntoskrnl.exe/pnp_manager.c > index 7ebd07d..04b218b 100644 > --- a/dlls/ntoskrnl.exe/pnp_manager.c > +++ b/dlls/ntoskrnl.exe/pnp_manager.c > @@ -17,6 +17,7 @@ > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > */ > > +#define NONAMELESSUNION > #include <stdarg.h> > #include "ntstatus.h" > #define WIN32_NO_STATUS > @@ -24,14 +25,207 @@ > #include "winbase.h" > #include "winternl.h" > #include "ddk/wdm.h" > +#include "wine/unicode.h" > #include "wine/debug.h" > > WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > > +#define MAX_SERVICE_NAME 260 > + > +static NTSTATUS WINAPI internal_complete(DEVICE_OBJECT *deviceObject, IRP *irp, void *context) > +{ > + SetEvent(irp->UserEvent); > + return STATUS_MORE_PROCESSING_REQUIRED; > +} > + > +static NTSTATUS send_device_irp(DEVICE_OBJECT *device, IRP *irp, ULONG_PTR *info) > +{ > + NTSTATUS status; > + IO_STACK_LOCATION *irpsp; > + HANDLE event = CreateEventA(NULL, FALSE, FALSE, NULL); > + > + irp->UserEvent = event; > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->CompletionRoutine = internal_complete; > + irpsp->Control = SL_INVOKE_ON_SUCCESS | SL_INVOKE_ON_ERROR; > + > + IoCallDriver(device, irp); > + > + if (irp->IoStatus.u.Status == STATUS_PENDING) > + WaitForSingleObject(event, INFINITE); > + > + status = irp->IoStatus.u.Status; > + if (info) > + *info = irp->IoStatus.Information; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + CloseHandle(event); > + return status; > +} > + > +static NTSTATUS get_device_id(DEVICE_OBJECT *device, BUS_QUERY_ID_TYPE type, WCHAR **id) > +{ > + IO_STACK_LOCATION *irpsp; > + IO_STATUS_BLOCK irp_status; > + IRP *irp; > + > + irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, device, NULL, 0, NULL, NULL, &irp_status); > + if (irp == NULL) > + return STATUS_NO_MEMORY; > + > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->MinorFunction = IRP_MN_QUERY_ID; > + irpsp->Parameters.QueryId.IdType = type; > + return send_device_irp(device, irp, (ULONG_PTR*)id); > +} > + > +static NTSTATUS find_driver_for_id(const WCHAR *id, WCHAR *driver) > +{ > + NTSTATUS rc; > + static const WCHAR critical_fmtW[] = > + {'\\','R','e','g','i','s','t','r','y', > + '\\','M','a','c','h','i','n','e', > + '\\','S','y','s','t','e','m', > + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', > + '\\','C','o','n','t','r','o','l', > + '\\','C','r','i','t','i','c','a','l','D','e','v','i','c','e','D','a','t','a','b','a','s','e', > + '\\','%','s',0}; > + > + HANDLE hkey; > + WCHAR *regW; > + UNICODE_STRING key; > + OBJECT_ATTRIBUTES attr; > + > + regW = HeapAlloc(GetProcessHeap(), 0, sizeof(critical_fmtW) + strlenW(id) * sizeof(WCHAR)); > + sprintfW(regW, critical_fmtW, id); > + RtlInitUnicodeString(&key, regW); > + > + InitializeObjectAttributes(&attr, &key, OBJ_CASE_INSENSITIVE, NULL, NULL); > + rc = ZwOpenKey(&hkey, KEY_ALL_ACCESS, &attr); > + if (rc == STATUS_SUCCESS) > + { > + static const WCHAR serviceW[] = {'S','e','r','v','i','c','e',0}; > + static const UNICODE_STRING serviceStr = {sizeof(serviceW) - sizeof(WCHAR), sizeof(serviceW), (WCHAR*)serviceW}; > + KEY_VALUE_PARTIAL_INFORMATION *info; > + DWORD total_size = sizeof(*info) + MAX_SERVICE_NAME * sizeof(WCHAR); > + > + info = HeapAlloc(GetProcessHeap(), 0, total_size); > + > + rc = ZwQueryValueKey(hkey, &serviceStr, KeyValuePartialInformation, info, total_size, &total_size); > + if (rc == STATUS_SUCCESS && info->Type == REG_SZ && info->DataLength < MAX_SERVICE_NAME * sizeof(WCHAR)) > + { > + WCHAR *service = (WCHAR*)info->Data; > + TRACE("Got service %s\n",debugstr_w(service)); > + strcpyW(driver, service); > + } > + else > + { > + if (info->Type != REG_SZ) > + ERR("Failed to get Service from registry entry, Bad type\n"); > + else if (info->DataLength >= MAX_SERVICE_NAME * sizeof(WCHAR)) > + ERR("Failed to get Service from registry entry, Buffer too small\n"); > + else > + ERR("Failed to get Service from registry entry (%x)\n",rc); I don't think it makes sense to trace this an error, especially when you use a TRACE for the lack of a critical device database key. > + } > + HeapFree(GetProcessHeap(), 0, info); > + ZwClose(hkey); > + } > + else > + TRACE("Failed to find Critical Device Database entry for %s\n",debugstr_w(id)); > + HeapFree(GetProcessHeap(), 0, regW); > + return rc; > +} > + > +static void handle_bus_relations(DEVICE_OBJECT *device) > +{ > + static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0}; > + static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', > + '\\','M','a','c','h','i','n','e', > + '\\','S','y','s','t','e','m', > + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', > + '\\','S','e','r','v','i','c','e','s', > + '\\',0}; > + > + NTSTATUS status; > + WCHAR *id, *idptr; > + WCHAR driver[MAX_SERVICE_NAME] = {0}; > + WCHAR fullname[MAX_PATH]; > + DRIVER_OBJECT *driver_obj; > + UNICODE_STRING driverName; > + WCHAR driverKey[MAX_PATH]; > + UNICODE_STRING driverPath; You'll have to allocate bigger buffers for the concatenated strings. In fact, a single buffer should be sufficient for both the fullname and the driver key. > + > + TRACE("Device %p\n", device); > + > + /* We could(should?) do a full IRP_MN_QUERY_DEVICE_RELATIONS query, > + * but we dont have to, We have the DEVICE_OBJECT of the new device > + * so we can simply handle the process here */ > + > + status = get_device_id(device, BusQueryCompatibleIDs, &id); > + if (status != ERROR_SUCCESS || !id) > + { > + ERR("Failed to get device IDs\n"); > + return; > + } > + > + idptr = id; > + while(*idptr != 0) > + { > + TRACE("Checking for id %s\n",debugstr_w(idptr)); > + if (find_driver_for_id(idptr, driver) == STATUS_SUCCESS) > + break; > + idptr += (strlenW(idptr) + 1); > + } > + HeapFree(GetProcessHeap(), 0, id); > + > + if (!driver[0]) > + { > + ERR("No matching driver found for device\n"); > + return; > + } > + > + strcpyW(driverKey, servicesW); > + strcatW(driverKey, driver); > + RtlInitUnicodeString(&driverPath, driverKey); > + if (ZwLoadDriver(&driverPath) != STATUS_SUCCESS) > + { > + ERR("Failed to load driver %s\n", debugstr_w(driver)); > + return; > + } > + > + strcpyW(fullname, driverW); > + strcatW(fullname, driver); > + RtlInitUnicodeString(&driverName, fullname); > + if (ObReferenceObjectByName(&driverName, OBJ_CASE_INSENSITIVE, NULL, > + 0, NULL, KernelMode, NULL, (void**)&driver_obj) != STATUS_SUCCESS) > + { > + ERR("Failed to locate loaded driver %s\n", debugstr_w(driver)); > + return; > + } > + > + if (!driver_obj->DriverExtension->AddDevice) > + status = STATUS_NOT_IMPLEMENTED; > + else > + status = driver_obj->DriverExtension->AddDevice(driver_obj, device); > + > + ObDereferenceObject(driver_obj); > + > + if (status != STATUS_SUCCESS) > + ERR("AddDevice failed for driver %s\n", debugstr_w(driver)); > +} > + > /*********************************************************************** > * IoInvalidateDeviceRelations(NTOSKRNL.EXE.@) > */ > void WINAPI IoInvalidateDeviceRelations(DEVICE_OBJECT *device_object, DEVICE_RELATION_TYPE type) > { > - FIXME("(%p, %i): stub\n", device_object, type); > + TRACE("(%p, %i)\n", device_object, type); > + > + switch (type) > + { > + case BusRelations: > + handle_bus_relations(device_object); > + break; > + default: > + FIXME("Unhandled Relation %i\n", type); > + } > } > diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h > index bd3b323..e893ba2 100644 > --- a/include/ddk/wdm.h > +++ b/include/ddk/wdm.h > @@ -48,6 +48,7 @@ typedef NTSTATUS (WINAPI *PDRIVER_INITIALIZE)(struct _DRIVER_OBJECT *, PUNICODE_ > typedef NTSTATUS (WINAPI *PDRIVER_DISPATCH)(struct _DEVICE_OBJECT *, struct _IRP *); > typedef void (WINAPI *PDRIVER_STARTIO)(struct _DEVICE_OBJECT *, struct _IRP *); > typedef void (WINAPI *PDRIVER_UNLOAD)(struct _DRIVER_OBJECT *); > +typedef NTSTATUS (WINAPI *PDRIVER_ADD_DEVICE)(struct _DRIVER_OBJECT *, struct _DEVICE_OBJECT *); > > typedef struct _DISPATCHER_HEADER { > UCHAR Type; > @@ -336,7 +337,7 @@ typedef struct _DEVICE_RELATIONS *PDEVICE_RELATIONS; > > typedef struct _DRIVER_EXTENSION { > struct _DRIVER_OBJECT *DriverObject; > - PVOID AddDevice; > + PDRIVER_ADD_DEVICE AddDevice; > ULONG Count; > UNICODE_STRING ServiceKeyName; > } DRIVER_EXTENSION, *PDRIVER_EXTENSION; > > >

1 0

Re: [PATCH 2/2] winhttp: Also pass hostname to jsproxy
by Nikolay Sivov 31 Aug '16

31 Aug '16

On 31.08.2016 22:21, Andrew Eikum wrote: > > +static inline char *strdupWA_sized( const WCHAR *src, DWORD size ) > +{ > + char *dst = NULL; > + if (src) > + { > + int len = WideCharToMultiByte( CP_ACP, 0, src, size, NULL, 0, NULL, NULL ) + 1; > + if ((dst = heap_alloc( len ))) > + { > + WideCharToMultiByte( CP_ACP, 0, src, len, dst, size, NULL, NULL ); > + dst[len - 1] = 0; > + } > + } > + return dst; > +} > + This is usually called strndup* in Wine dlls. Also 'len' and 'size' seem to be misplaced, and I'm not sure it will do the right thing if for negative size, unless it's not meant to ever be negative.

1 0

Re: [PATCH 1/4] xinput1_3: Better trace all functions
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 31.08.2016 20:17, Bruno Jesus wrote: > - FIXME("(%d %p %p) Stub!\n", dwUserIndex, pDSoundRenderGuid, pDSoundCaptureGuid); > + FIXME("(index %u, render guid %p, capture guid %p) Stub!\n", index, render_guid, capture_guid); debugstr_guid() would be more appropriate for tracing GUIDs.

2 2

How to find out the source code function from compiled running address
by Bruno Jesus 31 Aug '16

31 Aug '16

Hi all. While trying to run a crashing application I used a debugger and found out the crash is inside wined3d, I then started looking for ways to convert that address back into what function it should be so I can breakpoint it and check. At first I got into http://www.nirsoft.net/utils/dll_export_viewer.html and it looked like what I needed but the program does not seem to work in Wine DLLs. Adding +d3d,+relay and checking the calls is awfully verbose, but I'm still trying it. Part of the function showing the addresses, last line crashes: wined3d.dll:7EBCEF19 mov eax, [ebp+8] wined3d.dll:7EBCEF1C mov eax, [eax+0EC54h] wined3d.dll:7EBCEF22 mov edx, [ebp-0Ch] wined3d.dll:7EBCEF25 shl edx, 2 wined3d.dll:7EBCEF28 add eax, edx wined3d.dll:7EBCEF2A mov eax, [eax] Is there anyway to find out where is this function in the source code? Best regards, Bruno

3 3

Re: [PATCH 1/1] gdiplus: Fix for the "graphics object has no HDC" issue in GdipDrawPath
by Vincent Povirk 31 Aug '16

31 Aug '16

We really need an implementation that doesn't rely on gdi32.

2 6

Re: [PATCH 2/4] ntoskrnl.exe: Use static initializer for device_drivers.
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 30.08.2016 21:30, Jacek Caban wrote: > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/ntoskrnl.c | 40 ++++++++-------------------------------- > 1 file changed, 8 insertions(+), 32 deletions(-) > Wouldn't it be better to use a macro similar to LIST_INIT() for that (to allow later changes of the internal structure)?

2 2

Re: [2/8] webservices: Implement WsAddCustomHeader.
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 31.08.2016 14:35, Hans Leidekker wrote: > + if (WsXmlStringEquals( desc->elementLocalName, &msg->header[i]->name, NULL ) && > + WsXmlStringEquals( desc->elementNs, &msg->header[i]->ns, NULL ) == S_OK) This looks suspicious because it does not match the follow up patch. are you missing a == S_OK maybe?

1 0

Re: [PATCH 3/4] ntoskrnl.exe: Send PnP and Power IRPs to plug and play devices
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 29.08.2016 20:49, Aric Stewart wrote: > The drivers AddDevice function will be well behaved and call > IoAttachDeviceToDeviceStack. The Plug and Play manager will look at > the bus device after AddDevice and if there is an AttachedDevice then > will proceed to call IRP_MN_START_DEVICE on the attached device. Then > a IRP_MJ_POWER/IRP_MN_SET_POWER to PowerDeviceD0. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/pnp_manager.c | 64 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > > > 0003-ntoskrnl.exe-Send-PnP-and-Power-IRPs-to-plug-and-play-.txt > > > diff --git a/dlls/ntoskrnl.exe/pnp_manager.c b/dlls/ntoskrnl.exe/pnp_manager.c > index f1f3109..912f5fd 100644 > --- a/dlls/ntoskrnl.exe/pnp_manager.c > +++ b/dlls/ntoskrnl.exe/pnp_manager.c > @@ -148,6 +148,60 @@ static NTSTATUS get_device_id(DEVICE_OBJECT *device, BUS_QUERY_ID_TYPE type, WCH > return status; > } > > +static NTSTATUS send_deviceIRP(DEVICE_OBJECT* device, IRP *irp) > +{ > + NTSTATUS status; > + IO_STACK_LOCATION *irpsp; > + HANDLE event = CreateEventA(NULL, FALSE, FALSE, NULL); > + > + irp->UserEvent = event; > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->CompletionRoutine = internalComplete; > + irpsp->Control = SL_INVOKE_ON_SUCCESS | SL_INVOKE_ON_ERROR; > + > + IoCallDriver(device, irp); > + > + if (irp->IoStatus.u.Status == STATUS_PENDING) > + WaitForSingleObject(event, INFINITE); > + > + status = irp->IoStatus.u.Status; > + IoCompleteRequest(irp, IO_NO_INCREMENT ); > + CloseHandle(event); > + return status; > +} Such a helper function could also be used to simplify get_device_id. > + > +static NTSTATUS send_pnpIRP(DEVICE_OBJECT *device, UCHAR minor) > +{ > + IO_STACK_LOCATION *irpsp; > + IO_STATUS_BLOCK irp_status; > + > + IRP *irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, device, NULL, 0, NULL, NULL, &irp_status); It would be better to handle failure of IoBuildSynchronousFsdRequest like in get_device_id. The same also applies to send_powerIRP() below. > + > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->MinorFunction = minor; > + > + irpsp->Parameters.StartDevice.AllocatedResources = NULL; > + irpsp->Parameters.StartDevice.AllocatedResourcesTranslated = NULL; > + > + return send_deviceIRP(device, irp); > +} > + > +static NTSTATUS send_powerIRP(DEVICE_OBJECT *device, DEVICE_POWER_STATE power) Please check if its possible to (partially) move this to PoRequestPowerIrp(). If there is not enough information we can keep it like that for now, but according to the description it sounds very similar. > +{ > + IO_STATUS_BLOCK irp_status; > + IO_STACK_LOCATION *irpsp; > + > + IRP *irp = IoBuildSynchronousFsdRequest(IRP_MJ_POWER, device, NULL, 0, NULL, NULL, &irp_status); > + > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->MinorFunction = IRP_MN_SET_POWER; > + > + irpsp->Parameters.Power.Type = DevicePowerState; > + irpsp->Parameters.Power.State.DeviceState = power; > + > + return send_deviceIRP(device, irp); > +} > + > static void handle_bus_relations(DEVICE_OBJECT *device) > { > static const WCHAR szDriverW[] = {'\\','D','r','i','v','e','r','\\',0}; > @@ -244,6 +298,16 @@ static void handle_bus_relations(DEVICE_OBJECT *device) > ERR("AddDevice failed\n"); > return; > } > + > + if (device->AttachedDevice) > + { > + send_pnpIRP(device->AttachedDevice, IRP_MN_START_DEVICE); > + send_powerIRP(device->AttachedDevice, PowerDeviceD0); > + } > + else > + { > + ERR("No attached device\n"); > + } > } > > static void initialize_driver_store(void) > > >

1 0

Re: [PATCH 2/4] ntoskrnl.exe: Implement loading plug and play devices
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 29.08.2016 20:49, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/pnp_manager.c | 150 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 150 insertions(+) > > > > 0002-ntoskrnl.exe-Implement-loading-plug-and-play-devices.txt > > > diff --git a/dlls/ntoskrnl.exe/pnp_manager.c b/dlls/ntoskrnl.exe/pnp_manager.c > index 5b42a0a..f1f3109 100644 > --- a/dlls/ntoskrnl.exe/pnp_manager.c > +++ b/dlls/ntoskrnl.exe/pnp_manager.c > @@ -17,6 +17,7 @@ > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > */ > > +#define NONAMELESSUNION > #include <stdarg.h> > #include "ntstatus.h" > #define WIN32_NO_STATUS > @@ -27,6 +28,7 @@ > #include "cfgmgr32.h" > #include "winsvc.h" > #include "winreg.h" > +#include "wine/unicode.h" > #include "wine/debug.h" > #include "wine/list.h" > > @@ -34,6 +36,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > > #define MAX_SERVICE_NAME 260 > > +typedef NTSTATUS WINAPI (*pAddDevice)(DRIVER_OBJECT *DriverObject, DEVICE_OBJECT *PhysicalDeviceObject); My Win10 header files contain DRIVER_EXTENSION with the appropriate definition, which would make the casts unnecessary. > + > typedef struct _device_driver { > struct list entry; > > @@ -105,6 +109,143 @@ static device_driver* register_device_driver(const WCHAR *drivername, const WCHA > return ptr; > } > > +static NTSTATUS WINAPI internalComplete(DEVICE_OBJECT *deviceObject, IRP *irp, > + void *context ) > +{ > + SetEvent(irp->UserEvent); > + return STATUS_MORE_PROCESSING_REQUIRED; > +} > + > +static NTSTATUS get_device_id(DEVICE_OBJECT *device, BUS_QUERY_ID_TYPE type, WCHAR **id) > +{ > + NTSTATUS status; > + IO_STACK_LOCATION *irpsp; > + IO_STATUS_BLOCK irp_status; > + IRP *irp; > + HANDLE event = CreateEventA(NULL, FALSE, FALSE, NULL); > + > + irp = IoBuildSynchronousFsdRequest(IRP_MJ_PNP, device, NULL, 0, NULL, NULL, &irp_status); > + if (irp == NULL) > + return STATUS_NO_MEMORY; The return path here will leak the event handle. The same issue was recently fixed in hidclass.sys. Also I'm wondering a bit, do we need to keep this identical code at both places? > + > + irp->UserEvent = event; > + irpsp = IoGetNextIrpStackLocation(irp); > + irpsp->MinorFunction = IRP_MN_QUERY_ID; > + irpsp->Parameters.QueryId.IdType = type; > + irpsp->CompletionRoutine = internalComplete; > + irpsp->Control = SL_INVOKE_ON_SUCCESS | SL_INVOKE_ON_ERROR; > + > + IoCallDriver(device, irp); > + > + if (irp->IoStatus.u.Status == STATUS_PENDING) > + WaitForSingleObject(event, INFINITE); > + > + *id = (WCHAR*)irp->IoStatus.Information; > + status = irp->IoStatus.u.Status; > + IoCompleteRequest(irp, IO_NO_INCREMENT ); > + CloseHandle(event); > + > + return status; > +} > + > +static void handle_bus_relations(DEVICE_OBJECT *device) > +{ > + static const WCHAR szDriverW[] = {'\\','D','r','i','v','e','r','\\',0}; > + > + NTSTATUS status; > + WCHAR *id, *idptr; > + device_driver *driver = NULL; > + WCHAR fullname[MAX_PATH]; > + DRIVER_OBJECT *driver_obj; > + UNICODE_STRING driverName; > + > + TRACE("Device %p\n", device); > + > + /* We could(should?) do a full IRP_MN_QUERY_DEVICE_RELATIONS query, > + * but we dont have to, We have the DEVICE_OBJECT of the new device > + * so we can simply handle the process here */ > + > + status = get_device_id(device, BusQueryCompatibleIDs, &id); > + if (status != ERROR_SUCCESS || !id) > + { > + ERR("Failed to get device IDs\n"); > + return; > + } > + > + idptr = id; > + do { > + device_driver *ptr; > + > + TRACE("Checking for id %s\n",debugstr_w(idptr)); > + > + /* Check loaded drivers */ > + LIST_FOR_EACH_ENTRY(ptr, &device_drivers, device_driver, entry) > + { > + int i; > + for (i = 0; i < ptr->id_count; i++) > + { > + if (lstrcmpW(idptr, ptr->matching_ids[i]) == 0) > + { > + driver = ptr; > + break; Is it intentional that this does not abort the outer loop? > + } > + } > + } > + idptr += (lstrlenW(idptr) + 1); > + } while (!driver && *idptr != 0); > + > + HeapFree(GetProcessHeap(), 0, id); > + > + if (!driver) > + { > + ERR("No matching driver found for device\n"); > + return; > + } > + > + strcpyW(fullname, szDriverW); > + strcatW(fullname, driver->driver_name); > + RtlInitUnicodeString(&driverName, fullname); > + > + if (!driver->loaded) > + { > + static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', > + '\\','M','a','c','h','i','n','e', > + '\\','S','y','s','t','e','m', > + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', > + '\\','S','e','r','v','i','c','e','s', > + '\\',0}; > + WCHAR driverKey[MAX_PATH]; > + UNICODE_STRING driverPath; > + > + strcpyW(driverKey, servicesW); > + strcatW(driverKey, driver->driver_name); > + RtlInitUnicodeString(&driverPath, driverKey); > + if (ZwLoadDriver(&driverPath) != STATUS_SUCCESS) > + { > + ERR("Failed to load driver %s\n",debugstr_w(driver->driver_name)); > + return; > + } > + driver->loaded = TRUE; > + } > + > + if (ObReferenceObjectByName(&driverName, OBJ_CASE_INSENSITIVE, NULL, > + 0, NULL, KernelMode, NULL, (void**)&driver_obj) != STATUS_SUCCESS) > + { > + ERR("Failed to locate loaded driver\n"); > + return; > + } > + > + status = ((pAddDevice)driver_obj->DriverExtension->AddDevice)(driver_obj, device); It would be better to handle the lack of a AddDevice routine. > + > + ObDereferenceObject(driver_obj); > + > + if (status != STATUS_SUCCESS) > + { > + ERR("AddDevice failed\n"); > + return; > + } > +} > + > static void initialize_driver_store(void) > { > static const WCHAR criticalW[] = > @@ -181,5 +322,14 @@ VOID WINAPI IoInvalidateDeviceRelations(DEVICE_OBJECT *DeviceObject, DEVICE_RELA > initialized = TRUE; > initialize_driver_store(); > } > + > + switch (Type) > + { > + case BusRelations: > + handle_bus_relations(DeviceObject); > + break; > + default: > + FIXME("Unhandled Relation %i\n", Type); > + } > LeaveCriticalSection(&pnp_cs); > } > > >

1 0

Re: [PATCH 1/4] ntoskrnl.exe: Implement IoInvalidateDeviceRelations to initialize device driver store
by Sebastian Lackner 31 Aug '16

31 Aug '16

On 29.08.2016 20:49, Aric Stewart wrote: > We are not handling installing drivers just yet, so we just make use of the > critical device store. These are drivers that, on Windows, are automatically > loaded on boot every time. This database is maintained in > [CurrentControlSet/Control/CriticalDeviceDatabase] The keys in this registry key > represent a HardwareID that match a device. We query the IDs from the bus > device’s BusQueryHardwareIDs and if we find a match (from most specific to most > general) we look at that registry key. The CriticalDeviceDatabase entry > specify a [Service] value representing the driver. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/Makefile.in | 3 +- > dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- > dlls/ntoskrnl.exe/pnp_manager.c | 185 ++++++++++++++++++++++++++++++++++++ > include/ddk/wdm.h | 1 + > 4 files changed, 189 insertions(+), 2 deletions(-) > create mode 100644 dlls/ntoskrnl.exe/pnp_manager.c > At some places the patch does not really use a consistent style yet (for example VOID <-> void, placement of "*" inbetween of type and variable name, "sz" prefix for variables, ...), however I will focus mainly on the technical aspects in this review. > > > 0001-ntoskrnl.exe-Implement-IoInvalidateDeviceRelations-to-.txt > > > diff --git a/dlls/ntoskrnl.exe/Makefile.in b/dlls/ntoskrnl.exe/Makefile.in > index b459c54..2fa768b 100644 > --- a/dlls/ntoskrnl.exe/Makefile.in > +++ b/dlls/ntoskrnl.exe/Makefile.in > @@ -4,6 +4,7 @@ IMPORTS = advapi32 > > C_SRCS = \ > instr.c \ > - ntoskrnl.c > + ntoskrnl.c \ > + pnp_manager.c > > RC_SRCS = ntoskrnl.rc > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > index 119d406..c94b902 100644 > --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > @@ -400,7 +400,7 @@ > @ stdcall IoInitializeIrp(ptr long long) > @ stdcall IoInitializeRemoveLockEx(ptr long long long long) > @ stdcall IoInitializeTimer(ptr ptr ptr) > -@ stub IoInvalidateDeviceRelations > +@ stdcall IoInvalidateDeviceRelations(ptr long) > @ stub IoInvalidateDeviceState > @ stub IoIsFileOriginRemote > @ stub IoIsOperationSynchronous > diff --git a/dlls/ntoskrnl.exe/pnp_manager.c b/dlls/ntoskrnl.exe/pnp_manager.c > new file mode 100644 > index 0000000..5b42a0a > --- /dev/null > +++ b/dlls/ntoskrnl.exe/pnp_manager.c > @@ -0,0 +1,185 @@ > +/* HID plug and play Manager like functionality > + * > + * Copyright 2016 CodeWeavers, Aric Stewart > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > + > +#include <stdarg.h> > +#include "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windef.h" > +#include "winbase.h" > +#include "winternl.h" > +#include "ddk/wdm.h" > +#include "cfgmgr32.h" > +#include "winsvc.h" I don't think this include is required anymore. > +#include "winreg.h" > +#include "wine/debug.h" > +#include "wine/list.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > + > +#define MAX_SERVICE_NAME 260 > + > +typedef struct _device_driver { > + struct list entry; > + > + BOOL loaded; > + > + WCHAR **matching_ids; > + INT id_count; > + > + WCHAR *driver_name; > +} device_driver; > + > +struct list device_drivers = LIST_INIT(device_drivers); > +static BOOL initialized = FALSE; > + > +static CRITICAL_SECTION pnp_cs; > +static CRITICAL_SECTION_DEBUG critsect_debug = > +{ > + 0, 0, &pnp_cs, > + { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList }, > + 0, 0, { (DWORD_PTR)(__FILE__ ": pnp_cs") } > +}; > +static CRITICAL_SECTION pnp_cs = { &critsect_debug, -1, 0, 0, 0, 0 }; > + > +static inline LPWSTR strdupW(LPCWSTR src) > +{ > + LPWSTR dest; > + > + if (!src) > + return NULL; > + > + dest = HeapAlloc(GetProcessHeap(), 0, (lstrlenW(src) + 1) * sizeof(WCHAR)); > + if (dest) > + lstrcpyW(dest, src); For new code it would be better to follow the current Wine style guidelines, which means no LP* types and strlenW/strcpyW without l prefix. > + > + return dest; > +} > + > +static device_driver* register_device_driver(const WCHAR *drivername, const WCHAR* match) > +{ > + device_driver *ptr; > + > + LIST_FOR_EACH_ENTRY(ptr, &device_drivers, device_driver, entry) > + { > + if (lstrcmpW(drivername, ptr->driver_name) == 0) > + { > + int i; > + /* Check matching ids */ > + for (i = 0; i < ptr->id_count; i++) > + if (lstrcmpW(match, ptr->matching_ids[i]) == 0) > + return ptr; > + /* Add a new matching id, this should be reletivly rare, > + no exponental growth needed */ > + ptr->matching_ids = HeapReAlloc(GetProcessHeap(), 0, ptr->matching_ids, sizeof(WCHAR*) * (ptr->id_count+1)); > + ptr->matching_ids[ptr->id_count] = strdupW(match); > + ptr->id_count++; If it is very rare, wouldn't it be better to add them as separate driver entries then? This would simplify the code and avoid nested loops at some places. The current way of organizing the driver db also has the disadvantage that the same "matching id" could be added for multiple drivers. > + > + return ptr; > + } > + } > + > + ptr = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*ptr)); > + ptr->driver_name = strdupW(drivername); > + ptr->loaded = FALSE; > + ptr->matching_ids = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR*)); > + ptr->matching_ids[0] = strdupW(match); > + ptr->id_count = 1; > + list_add_tail(&device_drivers, &ptr->entry); > + > + return ptr; > +} > + > +static void initialize_driver_store(void) > +{ > + static const WCHAR criticalW[] = > + { 'S','y','s','t','e','m','\\', > + 'C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t','\\', > + 'C','o','n','t','r','o','l', '\\', > + 'C','r','i','t','i','c','a','l','D','e','v','i','c','e','D','a','t','a','b','a','s','e', 0}; > + static const WCHAR serviceW[] = {'S','e','r','v','i','c','e',0}; > + HKEY hkey_root; > + WCHAR szMatch[MAX_DEVICE_ID_LEN]; > + int i; > + > + if (RegOpenKeyW(HKEY_LOCAL_MACHINE, criticalW, &hkey_root)) > + { > + TRACE("No drivers found\n"); > + return; > + } > + > + for (i = 0; TRUE; i++) > + { > + HKEY hkey_service; > + DWORD rc; > + device_driver *driver; > + DWORD len = 0; > + WCHAR szService[MAX_SERVICE_NAME]; > + > + rc = RegEnumKeyW(hkey_root, i, szMatch, MAX_DEVICE_ID_LEN); > + if (rc == ERROR_NO_MORE_ITEMS) > + break; > + > + if (rc != 0) > + { > + ERR("Error %d reading key %d - skipping\n", rc, i); > + continue; > + } > + > + if (RegOpenKeyW(hkey_root, szMatch, &hkey_service)) > + { > + ERR("Error opening key %s - skipping\n",debugstr_w(szMatch)); > + continue; > + } > + > + len = sizeof(szService); > + rc = RegQueryValueExW(hkey_service, serviceW, NULL, NULL, (BYTE*)szService, &len); It might be useful to check the type of the registry key here. > + if (rc != ERROR_SUCCESS) > + { > + ERR("Error querying service from key %s - skipping\n",debugstr_w(szMatch)); > + RegCloseKey(hkey_service); > + continue; > + } > + RegCloseKey(hkey_service); > + > + TRACE("Registering service '%s' to match '%s'\n", debugstr_w(szService), debugstr_w(szMatch)); > + > + driver = register_device_driver(szService, szMatch); > + if (!driver) > + ERR("failed to load driver %s\n", debugstr_w(szService)); The message is actually a bit misleading. It will only occur for duplicate entries, which should not happen anyway. > + > + } > + > + RegCloseKey(hkey_root); > +} > + > +/*********************************************************************** > + * IoInvalidateDeviceRelations(NTOSKRNL.EXE.@) > + */ > +VOID WINAPI IoInvalidateDeviceRelations(DEVICE_OBJECT *DeviceObject, DEVICE_RELATION_TYPE Type) > +{ > + TRACE("(%p %i)\n", DeviceObject, Type); > + > + EnterCriticalSection(&pnp_cs); > + if (!initialized) > + { > + initialized = TRUE; > + initialize_driver_store(); > + } > + LeaveCriticalSection(&pnp_cs); > +} > diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h > index 68694af..bd3b323 100644 > --- a/include/ddk/wdm.h > +++ b/include/ddk/wdm.h > @@ -1233,6 +1233,7 @@ NTSTATUS WINAPI IoGetDeviceProperty(PDEVICE_OBJECT,DEVICE_REGISTRY_PROPERTY,ULO > PVOID WINAPI IoGetDriverObjectExtension(PDRIVER_OBJECT,PVOID); > PDEVICE_OBJECT WINAPI IoGetRelatedDeviceObject(PFILE_OBJECT); > void WINAPI IoInitializeIrp(IRP*,USHORT,CCHAR); > +void WINAPI IoInvalidateDeviceRelations(PDEVICE_OBJECT,DEVICE_RELATION_TYPE); > VOID WINAPI IoInitializeRemoveLockEx(PIO_REMOVE_LOCK,ULONG,ULONG,ULONG,ULONG); > NTSTATUS WINAPI IoWMIRegistrationControl(PDEVICE_OBJECT,ULONG); > > > >

1 0

Re: [PATCH 1/5] ddraw/tests: Move IDirect3DViewport::TransformVertices tests to ddraw1.c.
by Henri Verbeet 30 Aug '16

30 Aug '16

On 29 August 2016 at 23:01, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > + * is transformed, and this vertex is offscreen.· Trailing whitespace. As an aside, those float comparisons are probably safe enough, but perhaps those should use compare_vec4()?

1 0

Re: kernel32: Stub CAL_RETURN_GENITIVE_NAMES flag.
by Nikolay Sivov 30 Aug '16

30 Aug '16

On 29.08.2016 22:24, David Keijser wrote: > This stops GetCalendarInfoW from reporting unknown caltype when it's > really a case of unsupported flags. > > Signed-off-by: David Keijser <keijser(a)gmail.com> > --- > dlls/kernel32/time.c | 4 +++- > include/winnls.h | 7 ++++--- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/dlls/kernel32/time.c b/dlls/kernel32/time.c > index b7a7918..e1fd107 100644 > --- a/dlls/kernel32/time.c > +++ b/dlls/kernel32/time.c > @@ -860,6 +860,8 @@ int WINAPI GetCalendarInfoW(LCID Locale, CALID Calendar, CALTYPE CalType, > FIXME("flag CAL_NOUSEROVERRIDE used, not fully implemented\n"); > if (CalType & CAL_USE_CP_ACP) > FIXME("flag CAL_USE_CP_ACP used, not fully implemented\n"); > + if (CalType & CAL_RETURN_GENITIVE_NAMES) > + FIXME("flag CAL_RETURN_GENITIVE_NAMES used, not fully implemented\n"); Hi, David. LOCALE_RETURN_GENITIVE_NAMES itself is implemented, you should pass it to GetLocaleInfo instead of stripping it from caltype. P.S. anyone working on translations please take another look at SMONTHNAME* constants for your language, to make sure genitive forms are there, if applicable. > > if (CalType & CAL_RETURN_NUMBER) { > if (!lpValue) > @@ -878,7 +880,7 @@ int WINAPI GetCalendarInfoW(LCID Locale, CALID Calendar, CALTYPE CalType, > > /* FIXME: No verification is made yet wrt Locale > * for the CALTYPES not requiring GetLocaleInfoA */ > - switch (CalType & ~(CAL_NOUSEROVERRIDE|CAL_RETURN_NUMBER|CAL_USE_CP_ACP)) { > + switch (CalType & ~(CAL_NOUSEROVERRIDE|CAL_RETURN_NUMBER|CAL_USE_CP_ACP|CAL_RETURN_GENITIVE_NAMES)) { > case CAL_ICALINTVALUE: > if (CalType & CAL_RETURN_NUMBER) > return GetLocaleInfoW(Locale, LOCALE_RETURN_NUMBER | LOCALE_ICALENDARTYPE, > diff --git a/include/winnls.h b/include/winnls.h > index 1a7baaf..3d950f6 100644 > --- a/include/winnls.h > +++ b/include/winnls.h > @@ -471,9 +471,10 @@ static const WCHAR LOCALE_NAME_SYSTEM_DEFAULT[] = {'!','s','y','s','-','d','e',' > #define MAX_DEFAULTCHAR 2 > > /* Defines for calendar handling */ > -#define CAL_NOUSEROVERRIDE LOCALE_NOUSEROVERRIDE > -#define CAL_USE_CP_ACP LOCALE_USE_CP_ACP > -#define CAL_RETURN_NUMBER LOCALE_RETURN_NUMBER > +#define CAL_NOUSEROVERRIDE LOCALE_NOUSEROVERRIDE > +#define CAL_USE_CP_ACP LOCALE_USE_CP_ACP > +#define CAL_RETURN_NUMBER LOCALE_RETURN_NUMBER > +#define CAL_RETURN_GENITIVE_NAMES LOCALE_RETURN_GENITIVE_NAMES > > #define CAL_ICALINTVALUE 0x01 > #define CAL_SCALNAME 0x02 >

2 1

Re: [PATCH 5/5] ddraw/tests: Extend TransformVertices invalid data tests.
by Marvin 29 Aug '16

29 Aug '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25440 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 3/5] ddraw: Fix offscreen flag handling in TransformVertices.
by Marvin 29 Aug '16

29 Aug '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25439 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

Re: [PATCH 2/5] ddraw: Copy extra vertex data in viewport::TransformVertices.
by Marvin 29 Aug '16

29 Aug '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=25438 Your paranoid android. === w1064 (32 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768). === w1064 (64 bit ddraw1) === ddraw1.c:2215: Test failed: Expected (0,0)-(512,384), got (0,0)-(1024,768).

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel