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

Re: [PATCH (resend) 5/8] winebus.sys: IOCTL_HID_GET_STRING for iohid
by Ken Thomases 04 Nov '16

04 Nov '16

On Nov 3, 2016, at 7:15 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index 26243fb..6e3c044 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -88,6 +88,7 @@ > #include "winternl.h" > #include "winioctl.h" > #include "ddk/wdm.h" > +#include "ddk/hidtypes.h" > #include "wine/debug.h" > > #include "bus.h" > @@ -143,7 +144,28 @@ static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD > > static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > { > - return STATUS_NOT_IMPLEMENTED; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + CFStringRef str = NULL; It's not necessary to initialize str here. > + switch (index) > + { > + case HID_STRING_ID_IPRODUCT: > + str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDProductKey)); > + break; > + case HID_STRING_ID_IMANUFACTURER: > + str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDManufacturerKey)); > + break; > + case HID_STRING_ID_ISERIALNUMBER: > + str = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDSerialNumberKey)); > + break; > + default: > + ERR("Unknown string index\n"); > + return STATUS_NOT_IMPLEMENTED; > + } > + > + if (str) > + CFStringToWSTR(str, buffer, length); What if str is NULL? Should the function return a different result code, or should it set the caller's buffer to the empty string, or…? Certainly, it shouldn't just return success but leave the caller's buffer with junk. > + > + return STATUS_SUCCESS; > }

1 0

Re: [PATCH (resend) 2/8] winebus.sys: Implement adding IOHID devices
by Ken Thomases 04 Nov '16

04 Nov '16

On Nov 3, 2016, at 7:14 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 102 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 101 insertions(+), 1 deletion(-) > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index 882b483..f3e282c 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -98,14 +98,114 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > 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); > + wstr[len] = 0; > +} > + > +static DWORD CFNumberToDWORD(CFNumberRef num) > +{ > + DWORD dwNum = 0; > + if (num) > + CFNumberGetValue(num, kCFNumberIntType, &dwNum); The local should be of type "int" to match kCFNumberIntType. > + return dwNum; > +} > + … > + > +static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef 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, 256); You should use sizeof(serial_string) / sizeof(WCHAR) instead of 256 here. > + > + 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; > + IoInvalidateDeviceRelations(device, BusRelations); > + } > +} > + > /* This puts the relevent run loop for event handleing into a WINE thread */ > static DWORD CALLBACK runloop_thread(VOID *args) > { > CFRunLoopRef run_loop; > > + DRIVER_OBJECT *driver = (DRIVER_OBJECT*)args; > run_loop = CFRunLoopGetCurrent(); Minor style nit: the variable declarations would normally be grouped and separated from statements. Of course, the declaration and setting of run_loop could be merged into one line (in the previous patch) to eliminate the distinction. > > IOHIDManagerSetDeviceMatching(hid_manager, NULL); > + IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver); > IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess) > { > @@ -130,7 +230,7 @@ NTSTATUS WINAPI iohid_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registr > 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, NULL, 0, NULL))) > + if (!(run_loop_handle = CreateThread(NULL, 0, runloop_thread, driver, 0, NULL))) > { > ERR("Failed to initialize IOHID Manager thread\n"); > iohid_driver_obj = NULL; > >

1 0

Re: [PATCH v2 1/8] winebus.sys: Add OS/X IOHID bus
by Ken Thomases 04 Nov '16

04 Nov '16

On Nov 3, 2016, at 7:14 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > > > v2: remove Unload code as it will never be triggered > 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 | 151 +++++++++++++++++++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 3 + > 4 files changed, 157 insertions(+), 1 deletion(-) > create mode 100644 dlls/winebus.sys/bus_iohid.c > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > new file mode 100644 > index 0000000..882b483 > --- /dev/null > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -0,0 +1,151 @@ … > + > +WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +#ifdef HAVE_IOHIDMANAGERCREATE > + > +static DRIVER_OBJECT *iohid_driver_obj = NULL; > +static IOHIDManagerRef hid_manager; > + > +/* This puts the relevent run loop for event handleing into a WINE thread */ Spelling: "relevant", "handling". > +static DWORD CALLBACK runloop_thread(VOID *args) > +{ > + CFRunLoopRef run_loop; > + > + run_loop = CFRunLoopGetCurrent(); > + > + IOHIDManagerSetDeviceMatching(hid_manager, NULL); > + 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"); Under what circumstances might CFRunLoopRun() return (if any)? You should presumably unschedule the hid_manager from the run loop and release it in this path, too. > + 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, NULL, 0, NULL))) > + { > + ERR("Failed to initialize IOHID Manager thread\n"); > + iohid_driver_obj = NULL; You should CFRelease(hid_manager) in this failure path. > + return STATUS_UNSUCCESSFUL; > + } > + You should CloseHandle(run_loop_handle) before returning. > + 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; > } In general, this patch is a bit weird in that it doesn't do anything. It could maybe be combined with the next one. *shrug* -Ken

1 0

Re: [PATCH 1/2] rpcrt4/tests: Extend the tests for NdrDllGetClassObject().
by Marvin 03 Nov '16

03 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=26418 Your paranoid android. === w1064 (32 bit cstub) === cstub: unhandled exception c0000005 at 76CF3DFD === w864 (64 bit cstub) === cstub: unhandled exception c0000005 at 00007FFB7D5E2A12 === w1064 (64 bit cstub) === cstub: unhandled exception c0000005 at 00007FFC79AEDBF3

2 1

Re: [PATCH 2/5] d3d8/tests: Port test_check_device_format() from d3d9.
by Marvin 03 Nov '16

03 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=26400 Your paranoid android. === w8 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480. === w864 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480. === w1064 (32 bit device) === device.c:2794: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2799: Test failed: The focus window is active. device.c:2807: Test failed: Got unexpected hr 0x88760869. device.c:2811: Test failed: Got unexpect screen size 800x600. device.c:2826: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2835: Test failed: Got unexpect screen size 800x600. device.c:2848: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2856: Test failed: Got unexpect width 640. device.c:2857: Test failed: Got unexpect height 480.

1 0

Re: [PATCH 1/5] d3d9/tests: Add test for checking buffer resource format support.
by Marvin 03 Nov '16

03 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=26399 Your paranoid android. === w864 (64 bit device) === device.c:4392: Test failed: cw is 0xf60, expected 0x7f. device.c:4401: Test failed: Callback cw is 0xf60, expected 0x7f. === w1064 (64 bit device) === device.c:4392: Test failed: cw is 0xf60, expected 0x7f. device.c:4401: Test failed: Callback cw is 0xf60, expected 0x7f.

1 0

Re: libs/wine: give user an error message if WINEARCH is not win32/win64 (try 2)
by Austin English 03 Nov '16

03 Nov '16

On Sun, Oct 16, 2016 at 5:57 AM, Jens Reyer <jre.winesim(a)gmail.com> wrote: > Signed-off-by: Jens Reyer <jre.winesim(a)gmail.com> > --- > > lgtm and tested successfully. Thanks! Could I get some feedback or a reviewer assigned please? This has sat for a few weeks with a second signed-off but no other activity and it's close to dropping off of the tracker. -- -Austin GPG: 14FB D7EA A041 937B

1 0

old Windows API books
by Marcus Meissner 02 Nov '16

02 Nov '16

Hi folks, On reducing my library I found two Windows books I want to get rid of, with information that is not just in MSDN. Matt Pietrek - Windows 95 System Programming Secrets, including original Floppy Disk. Seems that has a lot of lowlevel details. Andrew Schulman - Unauthorized Windows 95 (in german language) Not so many details, looks less interesting on paging through. Is anyone interested in those? I could bring them to wineconf. Ciao, Marcus

1 0

Re: [v2 PATCH 2/2] gdiplus: Implement GdipBitmapGetHistogram()
by Vincent Povirk 02 Nov '16

02 Nov '16

> +static void set_histogram_point_gray(ARGB color, UINT *ch0, UINT *ch1, UINT *ch2, UINT *ch3) > +{ > + ch0[(76 * ((color >> 16) & 0xff) + 150 * ((color >> 8) & 0xff) + 28 * (color & 0xff)) / 0xff]++; > +} Shouldn't the multipliers add up to 255 here?

2 1

Re: [PATCH 1/2] crypt32/tests: testing priorities and flags of stores in a collection; and whether certs are saved in registry endpoint
by Jacek Caban 02 Nov '16

02 Nov '16

Hi Donat, Sorry it took a while, I wanted to do some testing myself before reviewing. Patches look mostly good to me, I have a few comments. On 10/28/16 5:15 PM, Donat Enikeev wrote: > Superceeds: https://source.winehq.org/patches/data/127162 > > v1: > Test of priorities and flags of stores in a collection > v2: > Test whether certificates are actually saved in the registry for the most common pathes and one todo for HLCU\My saved in the AppData > > Signed-off-by: Donat Enikeev <donat(a)enikeev.net> > --- > dlls/crypt32/tests/Makefile.in | 2 +- > dlls/crypt32/tests/store.c | 288 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 289 insertions(+), 1 deletion(-) > > diff --git a/dlls/crypt32/tests/Makefile.in b/dlls/crypt32/tests/Makefile.in > index d84ca17..d96ad1c 100644 > --- a/dlls/crypt32/tests/Makefile.in > +++ b/dlls/crypt32/tests/Makefile.in > @@ -1,5 +1,5 @@ > TESTDLL = crypt32.dll > -IMPORTS = crypt32 advapi32 > +IMPORTS = crypt32 advapi32 user32 shlwapi shell32 > > C_SRCS = \ > base64.c \ > diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c > index e811fbe..7fedaa4 100644 > --- a/dlls/crypt32/tests/store.c > +++ b/dlls/crypt32/tests/store.c > @@ -23,6 +23,9 @@ > > #include <windef.h> > #include <winbase.h> > +#include <winuser.h> > +#include <shlobj.h> > +#include <shlwapi.h> > #include <winreg.h> > #include <winerror.h> > #include <wincrypt.h> > @@ -347,6 +350,287 @@ static const BYTE serializedStoreWithCert[] = { > 0x08,0x30,0x06,0x01,0x01,0xff,0x02,0x01,0x01,0x00,0x00,0x00,0x00,0x00,0x00, > 0x00,0x00,0x00,0x00,0x00,0x00 }; > > +static const struct > +{ > + HKEY key; > + DWORD cert_store; > + BOOL todo_appdata_file; Please remove todo_ part of the name. > + WCHAR store_name[MAX_PATH]; MAX_PATH does not make much sense. I believe that hardcoding the size to a small number would be better here. If there is an overflow, compiler will warn about that anyway. > + LPCWSTR base_reg_path; const WCHAR * instead of LPCWSTR in new code would be slightly preferable. > +} reg_store_saved_certs[] = { > + { HKEY_LOCAL_MACHINE, CERT_SYSTEM_STORE_LOCAL_MACHINE, FALSE, > + {'R','O','O','T',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH }, > + { HKEY_LOCAL_MACHINE, CERT_SYSTEM_STORE_LOCAL_MACHINE, FALSE, > + {'M','Y',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH }, > + { HKEY_LOCAL_MACHINE, CERT_SYSTEM_STORE_LOCAL_MACHINE, FALSE, > + {'C','A',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH }, > + /* Adding to HKCU\Root triggers safety warning dialog at least on Win764 */ > + /* { HKEY_CURRENT_USER, CERT_SYSTEM_STORE_CURRENT_USER, > + {'R','O','O','T',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH }, */ > + { HKEY_CURRENT_USER, CERT_SYSTEM_STORE_CURRENT_USER, TRUE, > + {'M','Y',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH }, > + { HKEY_CURRENT_USER, CERT_SYSTEM_STORE_CURRENT_USER, FALSE, > + {'C','A',0}, CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH } > +}; > + > +/* Testing whether system stores are available for adding new certs > + * and checking directly in the registry whether they are actually saved or deleted. > + * However, by the moment Windows treats HKCU\My (at least) as a special case, > + * and uses AppData directory for storing certs, not registry > + * @see additionally testRegStore() verifying a content of read/saved certs > + */ > +static void testRegStoreSavedCerts(void) > +{ > + static const WCHAR fmt[] = > + { '%','s','\\','%','s','\\','%','s','\\','%','s',0}, > + ms_certs[] = > + { 'M','i','c','r','o','s','o','f','t','\\','S','y','s','t','e','m','C','e','r','t','i','f','i','c','a','t','e','s',0}, > + certs[] = > + {'C','e','r','t','i','f','i','c','a','t','e','s',0}, > + bigCert_hash[] = { > + '6','E','3','0','9','0','7','1','5','F','D','9','2','3', > + '5','6','E','B','A','E','2','5','4','0','E','6','2','2', > + 'D','A','1','9','2','6','0','2','A','6','0','8',0}; > + PCCERT_CONTEXT cert1, cert2; > + HCERTSTORE store; > + HANDLE cert_file; > + HRESULT pathres; > + WCHAR key_name[MAX_PATH], appdata_path[MAX_PATH]; > + HKEY key; > + BOOL ret; > + DWORD res,i; > + > + for (i = 0; i < sizeof(reg_store_saved_certs) / sizeof(reg_store_saved_certs[0]); i++) > + { > + store = CertOpenStore(CERT_STORE_PROV_SYSTEM_REGISTRY_W,0,0, > + reg_store_saved_certs[i].cert_store, reg_store_saved_certs[i].store_name); > + if (!store) > + { > + win_skip("Insufficient privileges for the test %d, skipping\n", i); > + continue; This should be skip() instead of win_skip(). Once Wine will implement access rights, it will hit that skip as well. I think that this skip is valid only in local machine store, current user should always have sufficient rights, it should be expressed here to make tests more strict. Also, testing GetLastError() in this branch would be nice. Thanks, Jacek

1 0