November 2016 - Wine-devel - list.winehq.org

Re: [v2 PATCH] user32/msgbox: Support WM_COPY Message
by Nikolay Sivov 08 Nov '16

08 Nov '16

On 08.11.2016 13:44, Alistair Leslie-Hughes wrote: > Fixes: https://bugs.winehq.org/show_bug.cgi?id=17205 > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/user32/msgbox.c | 86 +++++++++++++++++++-- > dlls/user32/tests/dialog.c | 187 ++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 267 insertions(+), 6 deletions(-) > > diff --git a/dlls/user32/msgbox.c b/dlls/user32/msgbox.c > index 2ba98c9..4735b8a 100644 > --- a/dlls/user32/msgbox.c > +++ b/dlls/user32/msgbox.c > @@ -44,6 +44,11 @@ struct ThreadWindows > HWND *handles; > }; > > +/* Index the order the buttons need to appear to an ID* constant */ > +static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY, > + IDCANCEL, IDIGNORE, IDTRYAGAIN, > + IDCONTINUE, IDHELP }; > + > static BOOL CALLBACK MSGBOX_EnumProc(HWND hwnd, LPARAM lParam) > { > struct ThreadWindows *threadWindows = (struct ThreadWindows *)lParam; > @@ -77,11 +82,6 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb) > WCHAR *buffer = NULL; > const WCHAR *ptr; > > - /* Index the order the buttons need to appear to an ID* constant */ > - static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY, > - IDCANCEL, IDIGNORE, IDTRYAGAIN, > - IDCONTINUE, IDHELP }; > - > nclm.cbSize = sizeof(nclm); > SystemParametersInfoW (SPI_GETNONCLIENTMETRICS, 0, &nclm, 0); > > @@ -319,6 +319,77 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb) > HeapFree( GetProcessHeap(), 0, buffer ); > } > > +static void MSGBOX_CopyToClipboard( HWND hwnd ) > +{ > + int i; > + static const WCHAR line[] = {'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-', > + '-','-','-','-','-','-','-','-','\r','\n', 0}; > + static const WCHAR crlf[] = {'\r','\n', 0}; > + static const WCHAR spaces[] = {' ',' ',' ', 0}; > + int lenTitle = GetWindowTextLengthW(hwnd) + 1; > + int lenMsg = GetWindowTextLengthW(GetDlgItem(hwnd, MSGBOX_IDTEXT)) + 1; > + > + /* > + --------------------------- > + Dialog Title > + --------------------------- > + Dialog Message > + --------------------------- > + Button(s) Text. OK > + --------------------------- > + */ > + int len = ((sizeof(crlf) * 3) + (sizeof(line) * 4) + lenTitle + lenMsg) * sizeof(WCHAR); This will allocate more than you need. > + WCHAR *text = HeapAlloc( GetProcessHeap(), 0, len); Maybe return right here if allocation failed? Also please use consistent formatting regarding spaces. > + if(text) > + { > + lstrcpyW(text, line); > + if (GetWindowTextW(hwnd, text + lstrlenW(text), lenTitle)) > + { > + HGLOBAL hMem; > + WCHAR *data; > + > + lstrcatW(text, crlf); > + lstrcatW(text, line); > + GetWindowTextW(GetDlgItem(hwnd, MSGBOX_IDTEXT), text + lstrlenW(text), lenMsg); There is GetDlgItemText() for that. > + lstrcatW(text, crlf); > + lstrcatW(text, line); > + > + for (i = 0; i < (sizeof(buttonOrder) / sizeof(buttonOrder[0])); i++) > + { > + HWND hItem = GetDlgItem(hwnd, buttonOrder[i]); > + if (GetWindowLongW(hItem, GWL_STYLE) & WS_VISIBLE) > + { Does IsWindowVisible() work here? > + WCHAR buffer[1024] = {0}; This looks excessive for a button text string. > > case WM_COMMAND: > switch (LOWORD(wParam)) > diff --git a/dlls/user32/tests/dialog.c b/dlls/user32/tests/dialog.c > index a37c678..0bb7dca 100644 > --- a/dlls/user32/tests/dialog.c > +++ b/dlls/user32/tests/dialog.c > @@ -40,6 +40,7 @@ > #include "winbase.h" > #include "wingdi.h" > #include "winuser.h" > +#include "winnls.h" > > #define MAXHWNDS 1024 > static HWND hwnd [MAXHWNDS]; > @@ -1392,6 +1393,187 @@ static void test_MessageBoxFontTest(void) > DestroyWindow(hDlg); > } > > +static const char msgbox_title[] = "%5!z9ZXw*ia;57n/FGl.bCH,Su\"mfKN;foCqAU\'j6AmoJgAc_D:Z0A\'E6PF_O/w"; What's the idea behind such title? If you're trying to create a unique window title, I don't think it has to be that unique. > +static WCHAR expectedOK[] = > +{ > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'%','5','!','z','9','Z','X','w','*','i','a',';','5','7','n','/','F','G','l','.','b','C','H',',','S','u','"','m','f', > +'K','N',';','f','o','C','q','A','U','\'','j','6','A','m','o','J','g','A','c','_','D',':','Z','0','A','\'','E','6','P', > +'F','_','O','/','w','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'M','e','s','s','a','g','e','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', > +'O','K',' ',' ',' ','\r','\n', > +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', 0 > +}; Instead of expected strings you can as well duplicate your implementation logic and generate expected result. This will help testing with localized dialogs. > + > +DWORD WINAPI WorkerThread(void *param) Please please use more specific name. > + { > + succeeded = lstrcmpW(expected, text) == 0; > + if(!succeeded) > + { > + ok(0, "%s\n", wine_dbgstr_w(text)); > + ok(0, "%s\n", wine_dbgstr_w(expected)); > + } > + } Could this simply be ok(!lstrcmpW(), "..."); ? > + > +static void test_MessageBox_WM_COPY_Test(void) > +{ > + DWORD tid = 0; > + > + non_english = (PRIMARYLANGID(GetUserDefaultLangID()) != LANG_ENGLISH); > + trace("non_english %d\n", non_english); > + > + CreateThread(NULL, 0, WorkerThread, &expectedOK, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_OK); > + > + CreateThread(NULL, 0, WorkerThread, &expectedOkCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_OKCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedAbortRetryIgnore, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_ABORTRETRYIGNORE); > + > + CreateThread(NULL, 0, WorkerThread, &expectedYesNo, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNO); > + > + CreateThread(NULL, 0, WorkerThread, &expectedYesNoCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNOCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedRetryCancel, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_RETRYCANCEL); > + > + CreateThread(NULL, 0, WorkerThread, &expectedCancelTryContinue, 0, &tid); > + MessageBoxA(NULL, "Message", msgbox_title, MB_CANCELTRYCONTINUE); > +} This could be a loop over possible button values.

1 0

Re: [PATCH] netapi32/test: Add trace to help narrow down failing test.
by Hans Leidekker 08 Nov '16

08 Nov '16

On Tue, 2016-11-08 at 10:07 +0000, Alistair Leslie-Hughes wrote: > Based off a test run with extra traces > https://testbot.winehq.org/JobDetails.pl?Key=26512&log_201=1#k201 > the intermittently failing test is in test_DavGetUNCFromHTTPPath. The failure is a timeout, so you're likely to find a random test. I think we should concentrate on the earlier tests and figure out where all that time is spent.

2 1

Re: [PATCH] user32/msgbox: Support WM_COPY Message
by Dmitry Timoshkov 08 Nov '16

08 Nov '16

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +static void MSGBOX_CopyToClipbaord( HWND hwnd ) A typo in the name. > + static const WCHAR carriage[] = {'\r','\n', 0}; A usual name would be crlf[W]. -- Dmitry.

1 0

Re: [PATCH] netapi32/test: Add trace to help narrow down failing test.
by Nikolay Sivov 08 Nov '16

08 Nov '16

On 08.11.2016 13:07, Alistair Leslie-Hughes wrote: > Based off a test run with extra traces > https://testbot.winehq.org/JobDetails.pl?Key=26512&log_201=1#k201 > the intermittently failing test is in test_DavGetUNCFromHTTPPath. > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/netapi32/tests/access.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dlls/netapi32/tests/access.c b/dlls/netapi32/tests/access.c > index f095d77..0fab0de 100644 > --- a/dlls/netapi32/tests/access.c > +++ b/dlls/netapi32/tests/access.c > @@ -688,6 +688,7 @@ static void test_DavGetUNCFromHTTPPath(void) > for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++) > { > buf[0] = 0; > + trace("test: %d\n", i); > size = tests[i].size; > ret = pDavGetUNCFromHTTPPath( tests[i].path, buf, &size ); > ok( ret == tests[i].ret, "%u: expected %u got %u\n", i, tests[i].ret, ret ); ok() lines already print failed test index, and in case of a crash manual test as you is enough.

1 0

Re: [PATCH] vbscript: Coerce datatype in StrComp
by Marvin 08 Nov '16

08 Nov '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=26507 Your paranoid android. === wxppro (32 bit) === Found no trace of the test summary line or of a crash === w2003std (32 bit) === Found no trace of the test summary line or of a crash === wvistau64 (32 bit) === Found no trace of the test summary line or of a crash === w2008s64 (32 bit) === Found no trace of the test summary line or of a crash === w7u (32 bit) === Found no trace of the test summary line or of a crash === w7pro64 (32 bit) === Found no trace of the test summary line or of a crash === w8 (32 bit) === Found no trace of the test summary line or of a crash === w864 (32 bit) === Found no trace of the test summary line or of a crash === w1064 (32 bit) === Found no trace of the test summary line or of a crash === wvistau64 (64 bit) === Found no trace of the test summary line or of a crash === w2008s64 (64 bit) === Found no trace of the test summary line or of a crash === w7pro64 (64 bit) === Found no trace of the test summary line or of a crash === w864 (64 bit) === Found no trace of the test summary line or of a crash === w1064 (64 bit) === Found no trace of the test summary line or of a crash

1 0

Constructive podium thumping at Wineconf?
by Jeremy White 08 Nov '16

08 Nov '16

Hey all, As is tradition, on Saturday I plan to review the winehq test regime, and pound my fist on the table until people commit to fixing tests. We've spent the past few years managing the transition to qemu. I've been using this page to triage that progress: https://www.winehq.org/~jwhite/latest.html That reflects issues with our tests on Windows on qemu, excluding Windows 8 and Windows 10 (although it looks like I could perhaps add Windows 8 back into the mix, as the bleeding does not seem that bad any longer). We've stalled out at about 13 issues; we don't seem to be able to get below that. (There are 8 'old' issues, and 5 new ones, in case anyone wants to go look, including ones from ddraw, kernel32, netapi32, and user32). Prior to the qemu drive, the original goal was to get at least one Linux machine known to run the tests cleanly. The further hope would be that if we had a known set of 'good' test machines, then we could require that every patch succeed when run on those machines. We've made sporadic progress on that, but we did purchase a set of hardware with the intent of making that hardware run those tests cleanly. I see that we've driven those down to about 20 per system. Those machines are 'portable', for generous definitions of portable. But we could bring one or both them to Wineconf and work directly on them if we liked. Would that be useful? And, more broadly, is there some other way to consider this that would be more useful? Is there useful prep we can do that will give way to more constructive sessions at Wineconf? Cheers, Jeremy

2 1

Re: [PATCH resend 5/7] winebus.sys: reports for iohid
by Sebastian Lackner 07 Nov '16

07 Nov '16

On 04.11.2016 13:46, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > v2-0005-winebus.sys-reports-for-iohid.txt > > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index e1a97ef..7208117 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -119,6 +119,13 @@ static DWORD CFNumberToDWORD(CFNumberRef num) > return dwNum; > } > > +static void handle_IOHIDDeviceIOHIDReportCallback(void * inContext, > + IOReturn inResult, void * inSender, IOHIDReportType inType, > + uint32_t inReportID, uint8_t * inReport, CFIndex inReportLength) I would suggest to use parameter names without "in" prefix. Also, there usually shouldn't be any space between "*" and the following variable name. > +{ It might be better to store the DEVICE_OBJECT in a local variable before passing it to the next function, to ensure the types are checked correctly. Otherwise it remains unnoticed when the signature of process_hid_report changes in the future. > + process_hid_report(inContext, inReport, inReportLength); > +} > + > static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > { > ULONG_PTR dev1 = (ULONG_PTR)*(IOHIDDeviceRef*)get_platform_private(device); > @@ -179,7 +186,17 @@ static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DW > > static NTSTATUS begin_report_processing(DEVICE_OBJECT *device) > { > - return STATUS_NOT_IMPLEMENTED; > + DWORD length; > + uint8_t *buffer; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + CFNumberRef num; To be compatible with the rest of the code, this should only start processing once. You might have to store the state of report processing in the platform struct. > + > + num = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDMaxInputReportSizeKey)); > + length = CFNumberToDWORD(num); > + buffer = HeapAlloc(GetProcessHeap(), 0, length); You might want to do some error checking here. > + > + IOHIDDeviceRegisterInputReportCallback(dev, buffer, length, handle_IOHIDDeviceIOHIDReportCallback, device); > + return STATUS_SUCCESS; > } > > static NTSTATUS set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > @@ -244,6 +261,9 @@ static void handle_RemovalCallback(void *inContext, IOReturn inResult, void *inS > { > DEVICE_OBJECT *device; > TRACE("OS/X IOHID Device Removed %p\n", inIOHIDDeviceRef); > + IOHIDDeviceRegisterInputReportCallback(inIOHIDDeviceRef, NULL, 0, NULL, NULL); > + /* Note: Yes, we leak the buffer. But according to research there is no > + safe way to deallocate that buffer. */ Is it a problem when IOHIDDeviceRegisterInputReportCallback is called, although report processing was never started? And why isn't it safe to deallocate the buffer here? I don't think you will get additional reports when the device has been removed. > device = bus_find_hid_device(&iohid_vtbl, inIOHIDDeviceRef); > if (device) > { > > >

2 1

Re: [PATCH v3 1/7] winebus.sys: Add OS/X IOHID bus
by Sebastian Lackner 07 Nov '16

07 Nov '16

On 04.11.2016 13:45, Aric Stewart wrote: > v2: remove Unload code as it will never be triggered > v3: Suggestions from Ken Thomases > Merge Implement adding IOHID devices into this patch > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/Makefile.in | 3 +- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_iohid.c | 255 +++++++++++++++++++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 3 + > 4 files changed, 261 insertions(+), 1 deletion(-) > create mode 100644 dlls/winebus.sys/bus_iohid.c > > > > v2-0001-winebus.sys-Add-OS-X-IOHID-bus.txt > > > diff --git a/dlls/winebus.sys/Makefile.in b/dlls/winebus.sys/Makefile.in > index 9d3a0c2..ac02533 100644 > --- a/dlls/winebus.sys/Makefile.in > +++ b/dlls/winebus.sys/Makefile.in > @@ -1,9 +1,10 @@ > MODULE = winebus.sys > IMPORTS = ntoskrnl setupapi > -EXTRALIBS = $(UDEV_LIBS) > +EXTRALIBS = $(IOKIT_LIBS) $(UDEV_LIBS) > EXTRAINCL = $(UDEV_CFLAGS) > EXTRADLLFLAGS = -Wb,--subsystem,native > > C_SRCS = \ > + bus_iohid.c \ > bus_udev.c \ > main.c > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 45a65e2..effbd4a 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -18,6 +18,7 @@ > > /* Busses */ > NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN; > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) DECLSPEC_HIDDEN; > > /* Native device function table */ > typedef struct > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > new file mode 100644 > index 0000000..e47ed13 > --- /dev/null > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -0,0 +1,255 @@ > +/* Bus like function for mac HID devices > + * > + * 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 "config.h" > +#include "wine/port.h" > + > +#include <stdarg.h> > + > +#if defined(HAVE_IOKIT_HID_IOHIDLIB_H) > +#define DWORD UInt32 > +#define LPDWORD UInt32* > +#define LONG SInt32 > +#define LPLONG SInt32* > +#define E_PENDING __carbon_E_PENDING > +#define ULONG __carbon_ULONG > +#define E_INVALIDARG __carbon_E_INVALIDARG > +#define E_OUTOFMEMORY __carbon_E_OUTOFMEMORY > +#define E_HANDLE __carbon_E_HANDLE > +#define E_ACCESSDENIED __carbon_E_ACCESSDENIED > +#define E_UNEXPECTED __carbon_E_UNEXPECTED > +#define E_FAIL __carbon_E_FAIL > +#define E_ABORT __carbon_E_ABORT > +#define E_POINTER __carbon_E_POINTER > +#define E_NOINTERFACE __carbon_E_NOINTERFACE > +#define E_NOTIMPL __carbon_E_NOTIMPL > +#define S_FALSE __carbon_S_FALSE > +#define S_OK __carbon_S_OK > +#define HRESULT_FACILITY __carbon_HRESULT_FACILITY > +#define IS_ERROR __carbon_IS_ERROR > +#define FAILED __carbon_FAILED > +#define SUCCEEDED __carbon_SUCCEEDED > +#define MAKE_HRESULT __carbon_MAKE_HRESULT > +#define HRESULT __carbon_HRESULT > +#define STDMETHODCALLTYPE __carbon_STDMETHODCALLTYPE > +#define PAGE_SHIFT __carbon_PAGE_SHIFT > +#include <IOKit/IOKitLib.h> > +#include <IOKit/hid/IOHIDLib.h> > +#undef ULONG > +#undef E_INVALIDARG > +#undef E_OUTOFMEMORY > +#undef E_HANDLE > +#undef E_ACCESSDENIED > +#undef E_UNEXPECTED > +#undef E_FAIL > +#undef E_ABORT > +#undef E_POINTER > +#undef E_NOINTERFACE > +#undef E_NOTIMPL > +#undef S_FALSE > +#undef S_OK > +#undef HRESULT_FACILITY > +#undef IS_ERROR > +#undef FAILED > +#undef SUCCEEDED > +#undef MAKE_HRESULT > +#undef HRESULT > +#undef STDMETHODCALLTYPE > +#undef DWORD > +#undef LPDWORD > +#undef LONG > +#undef LPLONG > +#undef E_PENDING > +#undef PAGE_SHIFT > +#endif /* HAVE_IOKIT_HID_IOHIDLIB_H */ > + > +#define NONAMELESSUNION > + > +#include "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windef.h" > +#include "winbase.h" > +#include "winternl.h" > +#include "winioctl.h" > +#include "ddk/wdm.h" > +#include "wine/debug.h" > + > +#include "bus.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +#ifdef HAVE_IOHIDMANAGERCREATE > + > +static DRIVER_OBJECT *iohid_driver_obj = NULL; > +static IOHIDManagerRef hid_manager; > + > +static const WCHAR busidW[] = {'I','O','H','I','D',0}; > + > +#include "initguid.h" > +DEFINE_GUID(GUID_DEVCLASS_IOHID, 0x989D309D,0x0470,0x4E1A,0x89,0x38,0x50,0x1F,0x42,0xBD,0x9A,0xCD); > + > +static void CFStringToWSTR(CFStringRef cstr, LPWSTR wstr, int length) > +{ > + int len = min(CFStringGetLength(cstr), length-1); > + CFStringGetCharacters(cstr, CFRangeMake(0, len), wstr); Are you sure it doesn't cause a compiler warning? Winemac code always uses casts from WCHAR* to UniChar*. > + wstr[len] = 0; > +} > + > +static DWORD CFNumberToDWORD(CFNumberRef num) > +{ > + int dwNum = 0; > + if (num) > + CFNumberGetValue(num, kCFNumberIntType, &dwNum); > + return dwNum; > +} > + > +static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > +{ > + return 0; > +} > + > +static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS begin_report_processing(DEVICE_OBJECT *device) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) > +{ > + *read = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static const platform_vtbl iohid_vtbl = { Usually "{" is on the next line. > + compare_platform_device, > + get_reportdescriptor, > + get_string, > + begin_report_processing, > + set_output_report, > + get_feature_report, > + set_feature_report, > +}; > + > +static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef inIOHIDDeviceRef) Those parameter names sound a bit unusual. Why is there always an "in" prefix in front? And why are there two spaces in front of "inIOHIDDeviceRef"? > +{ > + DEVICE_OBJECT *device; > + DWORD vid, pid, version; > + CFStringRef str = NULL; > + WCHAR serial_string[256]; > + BOOL is_gamepad; > + > + TRACE("OS/X IOHID Device Added %p\n", inIOHIDDeviceRef); > + > + vid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVendorIDKey))); > + pid = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDProductIDKey))); > + version = CFNumberToDWORD(IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDVersionNumberKey))); > + str = IOHIDDeviceGetProperty(inIOHIDDeviceRef, CFSTR(kIOHIDSerialNumberKey)); > + if (str) CFStringToWSTR(str, serial_string, sizeof(serial_string) / sizeof(WCHAR)); > + > + is_gamepad = (IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_GamePad) || > + IOHIDDeviceConformsTo(inIOHIDDeviceRef, kHIDPage_GenericDesktop, kHIDUsage_GD_Joystick)); > + > + device = bus_create_hid_device(iohid_driver_obj, busidW, vid, pid, version, 0, str?serial_string:NULL, is_gamepad, &GUID_DEVCLASS_IOHID, &iohid_vtbl, sizeof(inIOHIDDeviceRef)); > + if (!device) > + ERR("Failed to create device\n"); > + else > + { > + IOHIDDeviceRef* ext = get_platform_private(device); > + *ext = inIOHIDDeviceRef; I think it would be better to use a struct, even if you only want to store one value at the moment. Also, please use a helper function to avoid casts in the rest of the driver code, similar to how it is done for udev. > + IoInvalidateDeviceRelations(device, BusRelations); > + } > +} > + > +/* This puts the relevant run loop for event handling into a WINE thread */ > +static DWORD CALLBACK runloop_thread(VOID *args) Usually you should always use lowercase "void" in Wine code. > +{ > + DRIVER_OBJECT *driver = (DRIVER_OBJECT*)args; > + CFRunLoopRef run_loop = CFRunLoopGetCurrent(); > + > + IOHIDManagerSetDeviceMatching(hid_manager, NULL); > + IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver); There is no need to pass "driver" as context when you don't use it in the callback. > + IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > + if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess) > + { > + ERR("Couldn't open IOHIDManager.\n"); > + IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > + CFRelease(hid_manager); > + return 0; > + } > + > + CFRunLoopRun(); > + TRACE("Run Loop exiting\n"); > + > + IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, NULL, NULL); > + IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); Can you be sure that run_loop is still valid here? > + CFRelease(hid_manager); > + return 1; > +} > + > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) > +{ > + HANDLE run_loop_handle; > + > + TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer)); > + > + iohid_driver_obj = driver; > + driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; > + hid_manager = IOHIDManagerCreate(kCFAllocatorDefault, 0L); > + if (!(run_loop_handle = CreateThread(NULL, 0, runloop_thread, driver, 0, NULL))) > + { > + ERR("Failed to initialize IOHID Manager thread\n"); > + iohid_driver_obj = NULL; > + CFRelease(hid_manager); > + return STATUS_UNSUCCESSFUL; > + } > + > + CloseHandle(run_loop_handle); > + return STATUS_SUCCESS; > +} > + > +#else > + > +NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry_path) > +{ > + WARN("IOHID Support not compiled into Wine.\n"); > + return STATUS_NOT_IMPLEMENTED; > +} > + > +#endif /* HAVE_IOHIDMANAGERCREATE */ > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 791ccbb..ed95435 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -628,10 +628,13 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > static UNICODE_STRING udev = {sizeof(udevW) - sizeof(WCHAR), sizeof(udevW), (WCHAR *)udevW}; > + static const WCHAR iohidW[] = {'\\','D','r','i','v','e','r','\\','I','O','H','I','D',0}; > + static UNICODE_STRING iohid = {sizeof(iohidW) - sizeof(WCHAR), sizeof(iohidW), (WCHAR *)iohidW}; > > TRACE( "(%p, %s)\n", driver, debugstr_w(path->Buffer) ); > > IoCreateDriver(&udev, udev_driver_init); > + IoCreateDriver(&iohid, iohid_driver_init); > > return STATUS_SUCCESS; > } > > >

2 1

Re: [PATCH 1/2] crypt32: Import system root certs into volatile registry keys instead of dedicated root store.
by Jacek Caban 07 Nov '16

07 Nov '16

It was me sending changed Donat's patches with messed From header. Sorry about that. Same for 2/2 patch. Jacek

2 3

Re: rsaenh/tests: Don't initialize the environment, the tests that follow do that.
by Marvin 07 Nov '16

07 Nov '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=26484 Your paranoid android. === w864 (32 bit rsaenh) === rsaenh.c:2804: Test failed: 80090020 === w1064 (32 bit rsaenh) === rsaenh.c:2804: Test failed: 80090020 === w1064 (64 bit rsaenh) === rsaenh.c:2804: Test failed: 80090020

3 3