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

Re: [v2 PATCH] user32/msgbox: Support WM_COPY Message
by Nikolay Sivov Nov. 8, 2016

Nov. 8, 2016

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 Nov. 8, 2016

Nov. 8, 2016

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 Nov. 8, 2016

Nov. 8, 2016

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 Nov. 8, 2016

Nov. 8, 2016

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 Nov. 8, 2016

Nov. 8, 2016

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 Nov. 8, 2016

Nov. 8, 2016

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 Nov. 7, 2016

Nov. 7, 2016

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 Nov. 7, 2016

Nov. 7, 2016

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 Nov. 7, 2016

Nov. 7, 2016

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 Nov. 7, 2016

Nov. 7, 2016

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

Re: [PATCH resend 7/7] winebus.sys: IOCTL_HID_GET_FEATURE for iohid
by Sebastian Lackner Nov. 7, 2016

Nov. 7, 2016

On 04.11.2016 13:47, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 33 +++++++++++++++++++++++++++++---- > 1 file changed, 29 insertions(+), 4 deletions(-) > > > > v2-0007-winebus.sys-IOCTL_HID_GET_FEATURE-for-iohid.txt > > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index 59c02ea..402451c 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -218,14 +218,39 @@ static NTSTATUS set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, > > static NTSTATUS get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) > { > - *read = 0; > - return STATUS_NOT_IMPLEMENTED; > + IOReturn ret; > + CFIndex report_length = length; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + > + ret = IOHIDDeviceGetReport(dev, kIOHIDReportTypeFeature, id, report, &report_length); > + if (ret == kIOReturnSuccess) > + { > + *read = report_length; > + return STATUS_SUCCESS; > + } > + else > + { > + *read = 0; > + return STATUS_UNSUCCESSFUL; > + } > } > > static NTSTATUS set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > { > - *written = 0; > - return STATUS_NOT_IMPLEMENTED; > + IOReturn result; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + > + result = IOHIDDeviceSetReport(dev, kIOHIDReportTypeFeature, id, report, length); The line above has 5 space indentation. > + if (result == kIOReturnSuccess) > + { > + *written = length; > + return STATUS_SUCCESS; > + } > + else > + { > + *written = 0; > + return STATUS_UNSUCCESSFUL; > + } > } > > static const platform_vtbl iohid_vtbl = { > > >

1 0

Re: [PATCH resend 3/7] winebus.sys: IOCTL_HID_GET_DEVICE_DESCRIPTOR for iohid
by Sebastian Lackner Nov. 7, 2016

Nov. 7, 2016

On 04.11.2016 13:46, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > v2-0003-winebus.sys-IOCTL_HID_GET_DEVICE_DESCRIPTOR-for-ioh.txt > > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index 3cd0b19..380cdb4 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -127,7 +127,18 @@ static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > > static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > { > - return STATUS_NOT_IMPLEMENTED; > + IOHIDDeviceRef dev = *(IOHIDDeviceRef*)get_platform_private(device); > + CFDataRef data = IOHIDDeviceGetProperty(dev, CFSTR(kIOHIDReportDescriptorKey)); Is it always guaranteed that this returns valid data? If not, it might be better to check the returned CFTypeRef. > + int data_length = CFDataGetLength(data); > + const UInt8 *ptr; > + > + *out_length = data_length; > + if (length < data_length) > + return STATUS_BUFFER_TOO_SMALL; > + > + ptr = CFDataGetBytePtr(data); > + memcpy(buffer, ptr, data_length); > + return STATUS_SUCCESS; > } > > static NTSTATUS get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > > >

1 0

Re: [PATCH v2 2/7] winebus.sys: Implement removing IOHID devices
by Sebastian Lackner Nov. 7, 2016

Nov. 7, 2016

On 04.11.2016 13:45, Aric Stewart wrote: > v2: Suggestions from Ken Thomases > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > v2-0002-winebus.sys-Implement-removing-IOHID-devices.txt > > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index e47ed13..3cd0b19 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -120,7 +120,9 @@ static DWORD CFNumberToDWORD(CFNumberRef num) > > static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > { > - return 0; > + ULONG_PTR dev1 = (ULONG_PTR)*(IOHIDDeviceRef*)get_platform_private(device); > + ULONG_PTR dev2 = (ULONG_PTR)platform_dev; > + return dev1 - dev2; As already mentioned, the int return value is not sufficient to hold the difference between the two ULONG_PTR values. > } > > static NTSTATUS get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > @@ -196,6 +198,18 @@ static void handle_DeviceMatchingCallback(void *inContext, IOReturn inResult, vo > } > } > > +static void handle_RemovalCallback(void *inContext, IOReturn inResult, void *inSender, IOHIDDeviceRef inIOHIDDeviceRef) Similar to patch 1, you could probably use better parameter names here. > +{ > + DEVICE_OBJECT *device; > + TRACE("OS/X IOHID Device Removed %p\n", inIOHIDDeviceRef); > + device = bus_find_hid_device(&iohid_vtbl, inIOHIDDeviceRef); > + if (device) > + { > + IoInvalidateDeviceRelations(device, RemovalRelations); > + bus_remove_hid_device(device); > + } > +} > + > /* This puts the relevant run loop for event handling into a WINE thread */ > static DWORD CALLBACK runloop_thread(VOID *args) > { > @@ -204,6 +218,7 @@ static DWORD CALLBACK runloop_thread(VOID *args) > > IOHIDManagerSetDeviceMatching(hid_manager, NULL); > IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, handle_DeviceMatchingCallback, driver); > + IOHIDManagerRegisterDeviceRemovalCallback(hid_manager, handle_RemovalCallback, driver); The context parameter is also not really used here. > IOHIDManagerScheduleWithRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > if (IOHIDManagerOpen( hid_manager, 0 ) != kIOReturnSuccess) > { > @@ -217,6 +232,7 @@ static DWORD CALLBACK runloop_thread(VOID *args) > TRACE("Run Loop exiting\n"); > > IOHIDManagerRegisterDeviceMatchingCallback(hid_manager, NULL, NULL); > + IOHIDManagerRegisterDeviceRemovalCallback(hid_manager, NULL, NULL); > IOHIDManagerUnscheduleFromRunLoop(hid_manager, run_loop, kCFRunLoopDefaultMode); > CFRelease(hid_manager); > return 1; > > >

1 0

Re: [PATCH 4/4] d2d1: Added support for D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT
by Henri Verbeet Nov. 7, 2016

Nov. 7, 2016

On 5 November 2016 at 22:12, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > @@ -1574,14 +1575,89 @@ static HRESULT STDMETHODCALLTYPE d2d_text_renderer_DrawGlyphRun(IDWriteTextRende > > if (desc) > WARN("Ignoring glyph run description %p.\n", desc); > - if (context->options & ~D2D1_DRAW_TEXT_OPTIONS_NO_SNAP) > + if (context->options & ~(D2D1_DRAW_TEXT_OPTIONS_NO_SNAP|D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT)) > FIXME("Ignoring options %#x.\n", context->options); "D2D1_DRAW_TEXT_OPTIONS_NO_SNAP | D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT" > + if (FAILED(hr = DWriteCreateFactory(DWRITE_FACTORY_TYPE_SHARED, &IID_IDWriteFactory2, > + (IUnknown **)&dwrite_factory))) > + { > + ERR("Failed to create dwrite factory, hr %#x.\n", hr); > + return hr; This leaks "brush". > + hr = IDWriteFactory2_TranslateColorGlyphRun(dwrite_factory, baseline_origin_x, baseline_origin_y, > + glyph_run, desc, measuring_mode, (DWRITE_MATRIX *)&render_target->drawing_state.transform, 0, &layers); > + IDWriteFactory2_Release(dwrite_factory); > + if (FAILED(hr)) > + { > + ERR("Failed to create color glyph run enumerator, hr %#x.\n", hr); > + return hr; > + } Likewise. > + while (1) for (;;) > + if (color_run->paletteIndex == 0xffff) > + color_brush = brush; > + else > + ID2D1RenderTarget_CreateSolidColorBrush(&render_target->ID2D1RenderTarget_iface, > + &color_run->runColor, NULL, (ID2D1SolidColorBrush **)&color_brush); CreateSolidColorBrush() can fail.

1 0

Re: [PATCH] vbscript: Call GetItemInfo() with original item name
by Jacek Caban Nov. 7, 2016

Nov. 7, 2016

Hi Nikolay, On 05.11.2016 22:53, Nikolay Sivov wrote: > diff --git a/programs/wscript/tests/run.js b/programs/wscript/tests/run.js > index db0d6f9..2d544ab 100644 > --- a/programs/wscript/tests/run.js > +++ b/programs/wscript/tests/run.js This deserves tests inside vbscript tests that shows the exact problem. I send a patch with such test, please send wscript tests as a separated patch. Thanks, Jacek

1 0

Re: [PATCH] user32: Add support for class versions
by Nikolay Sivov Nov. 7, 2016

Nov. 7, 2016

On 05.11.2016 16:36, Ivan Akulinchev wrote: > Signed-off-by: Ivan Akulinchev <ivan.akulinchev(a)gmail.com> > --- > dlls/comctl32/comctl32.manifest | 1 - > dlls/comctl32/theming.c | 15 +++++++-- > dlls/user32/class.c | 73 +++++++++++++++++++++++++++++++++++++---- > dlls/user32/user_private.h | 2 ++ > dlls/user32/win.c | 7 ++-- > 5 files changed, 86 insertions(+), 12 deletions(-) > Hi, thanks for looking into this. > diff --git a/dlls/comctl32/comctl32.manifest b/dlls/comctl32/comctl32.manifest > index 4c86137..5aa55a3 100644 > --- a/dlls/comctl32/comctl32.manifest > +++ b/dlls/comctl32/comctl32.manifest > @@ -12,7 +12,6 @@ > <windowClass>NativeFontCtl</windowClass> > <windowClass>ReBarWindow32</windowClass> > <windowClass>ScrollBar</windowClass> > - <windowClass>Static</windowClass> > <windowClass>SysAnimate32</windowClass> > <windowClass>SysDateTimePick32</windowClass> > <windowClass>SysHeader32</windowClass> Something tells me we'll need opposite logic here, if versioned classes support are to be added one by one. > diff --git a/dlls/comctl32/theming.c b/dlls/comctl32/theming.c > index 93d6fe6..b78ecc9 100644 > --- a/dlls/comctl32/theming.c > +++ b/dlls/comctl32/theming.c > @@ -28,6 +28,7 @@ > #include "comctl32.h" > #include "uxtheme.h" > #include "wine/debug.h" > +#include "wine/unicode.h" > > WINE_DEFAULT_DEBUG_CHANNEL(theming); > > @@ -124,8 +125,6 @@ void THEMING_Initialize (void) > static const WCHAR refDataPropName[] = > { 'C','C','3','2','T','h','e','m','i','n','g','D','a','t','a',0 }; > > - if (!IsThemeActive()) return; > - > atSubclassProp = GlobalAddAtomW (subclassPropName); > atRefDataProp = GlobalAddAtomW (refDataPropName); > > @@ -142,6 +141,18 @@ void THEMING_Initialize (void) > } > originalProcs[i] = class.lpfnWndProc; > class.lpfnWndProc = subclassProcs[i]; > + > + /* > + * FIXME: The Dialog class ('#32770') should NOT be overridden here. > + * > + * Temporarily ignore this issue using the hack below... > + */ > + if (strcmpiW(subclasses[i].className, dialogClass) != 0) > + { > + /* If not a dialog (see above), make the class global */ > + class.style |= CS_GLOBALCLASS; > + class.hInstance = NULL; > + } > > if (!class.lpfnWndProc) > { All theming.c should go when transition is complete, because once you're able to create 6.0.0.0!Static it will support theme drawing API regardless if theming is enabled or not. > diff --git a/dlls/user32/class.c b/dlls/user32/class.c > index 0b3582e..5b16008 100644 > --- a/dlls/user32/class.c > +++ b/dlls/user32/class.c > @@ -411,6 +411,42 @@ static CLASS *CLASS_RegisterClass( LPCWSTR name, HINSTANCE hInstance, BOOL local > return classPtr; > } > > +/*********************************************************************** > + * CLASS_GetVersionedName > + * > + * Return a versioned class name, like "6.0.2600.2982!Button". > + */ > +LPCWSTR CLASS_GetVersionedName( LPCWSTR name ) > +{ > + ACTCTX_SECTION_KEYED_DATA data = { sizeof(data) }; > + > + static const WCHAR staticW[] = { 'S','t','a','t','i','c',0 }; > + > + if (!name) > + return NULL; > + > + if (IS_INTRESOURCE( name )) > + return name; > + > + /* > + * FIXME: I disabled the Static class in comctl32.manifest (because it is > + * not implemented there yet), however I still get "6.0.2600.2982!Static". > + * > + * Is it a FindActCtxSectionString bug? > + */ > + if (strcmpiW( name, staticW ) == 0) > + return name; This needs separate investigation if it's truly happens without Static present in manifest. > + > + if (FindActCtxSectionStringW( 0, NULL, 3, name, &data )) > + { > + BYTE *res = (BYTE *)data.lpData; > + ULONG offset = *(ULONG *)(res + sizeof(ULONG) * 2 + sizeof(DWORD)); > + return (LPCWSTR)(res + offset); > + } > + > + return name; This makes sense, but please use types from ntdll/actctx.c (just duplicate what you need in user32, same as it's done in ole32 and kernel32 already). Also 3 here stands for ACTIVATION_CONTEXT_SECTION_WINDOW_CLASS_REDIRECTION. > +} > + > > /*********************************************************************** > * register_builtin > @@ -576,10 +612,12 @@ ATOM WINAPI RegisterClassExA( const WNDCLASSEXA* wc ) > > if (!IS_INTRESOURCE(wc->lpszClassName)) > { > + LPCWSTR versioned_name; > WCHAR name[MAX_ATOM_LEN + 1]; > > if (!MultiByteToWideChar( CP_ACP, 0, wc->lpszClassName, -1, name, MAX_ATOM_LEN + 1 )) return 0; > - classPtr = CLASS_RegisterClass( name, instance, !(wc->style & CS_GLOBALCLASS), > + versioned_name = CLASS_GetVersionedName( name ); > + classPtr = CLASS_RegisterClass( versioned_name, instance, !(wc->style & CS_GLOBALCLASS), > wc->style, wc->cbClsExtra, wc->cbWndExtra ); > } Why do you need this in -A call too? > else > @@ -619,6 +657,7 @@ ATOM WINAPI RegisterClassExW( const WNDCLASSEXW* wc ) > ATOM atom; > CLASS *classPtr; > HINSTANCE instance; > + LPCWSTR versioned_name; > > GetDesktopWindow(); /* create the desktop window to trigger builtin class registration */ > > @@ -630,7 +669,9 @@ ATOM WINAPI RegisterClassExW( const WNDCLASSEXW* wc ) > } > if (!(instance = wc->hInstance)) instance = GetModuleHandleW( NULL ); > > - if (!(classPtr = CLASS_RegisterClass( wc->lpszClassName, instance, !(wc->style & CS_GLOBALCLASS), > + versioned_name = CLASS_GetVersionedName( wc->lpszClassName ); > + > + if (!(classPtr = CLASS_RegisterClass( versioned_name, instance, !(wc->style & CS_GLOBALCLASS), > wc->style, wc->cbClsExtra, wc->cbWndExtra ))) > return 0; > This part I looks wrong. Why would it register something that is not what user asked for? My understanding is that comctl32 itself should register its versioned classes (and in ideal v5 vs v6 separation only v6 module should be doing that). So comctl32 is loaded, it registers regular classes first, like Listview, then it registered versioned names like 6.0.0.0!Listview. What RegisterClassExW should probably check is if target class in current context is already registered, so if application tries to register ClassName, and context has redirection set as 6.0.0.0!ClassName, registration should fail. I think it should be possible to add a test for that: - create temporary context with window class redirection section; - manually register corresponding versioned class name; - activate context; - see what happens if you try to register non-version class. I vaguely remember I tried something like that on Windows years ago, and as expected it breaks this whole redirection scheme, because if 6.x.x.x!Button is already registered by the time context is activated comctl32 won't be able to register it again. > @@ -678,6 +719,7 @@ BOOL WINAPI UnregisterClassA( LPCSTR className, HINSTANCE hInstance ) > BOOL WINAPI UnregisterClassW( LPCWSTR className, HINSTANCE hInstance ) > { > CLASS *classPtr = NULL; > + LPCWSTR versioned_name = CLASS_GetVersionedName( className ); > > GetDesktopWindow(); /* create the desktop window to trigger builtin class registration */ > > @@ -685,7 +727,7 @@ BOOL WINAPI UnregisterClassW( LPCWSTR className, HINSTANCE hInstance ) > { > req->instance = wine_server_client_ptr( hInstance ); > if (!(req->atom = get_int_atom_value(className)) && className) > - wine_server_add_data( req, className, strlenW(className) * sizeof(WCHAR) ); > + wine_server_add_data( req, versioned_name, strlenW(versioned_name) * sizeof(WCHAR) ); > if (!wine_server_call_err( req )) classPtr = wine_server_get_ptr( reply->client_ptr ); > } > SERVER_END_REQ; > @@ -1192,10 +1234,15 @@ BOOL WINAPI GetClassInfoExA( HINSTANCE hInstance, LPCSTR name, WNDCLASSEXA *wc ) > > if (!IS_INTRESOURCE(name)) > { > + LPCWSTR versioned_name; > WCHAR nameW[MAX_ATOM_LEN + 1]; > if (!MultiByteToWideChar( CP_ACP, 0, name, -1, nameW, sizeof(nameW)/sizeof(WCHAR) )) > return FALSE; > - classPtr = CLASS_FindClass( nameW, hInstance ); > + versioned_name = CLASS_GetVersionedName( nameW ); > + classPtr = CLASS_FindClass( versioned_name, hInstance ); > + /* FIXME: I believe we shouldn't fall back to the normal class. Fill > + * free to remove it if necessary. */ > + if (!classPtr) classPtr = CLASS_FindClass( nameW, hInstance ); > } > else classPtr = CLASS_FindClass( (LPCWSTR)name, hInstance ); > > @@ -1230,6 +1277,7 @@ BOOL WINAPI GetClassInfoExW( HINSTANCE hInstance, LPCWSTR name, WNDCLASSEXW *wc > { > ATOM atom; > CLASS *classPtr; > + LPCWSTR versioned_name; > > TRACE("%p %s %p\n", hInstance, debugstr_w(name), wc); > > @@ -1241,10 +1289,21 @@ BOOL WINAPI GetClassInfoExW( HINSTANCE hInstance, LPCWSTR name, WNDCLASSEXW *wc > > if (!hInstance) hInstance = user32_module; > > - if (!(classPtr = CLASS_FindClass( name, hInstance ))) > + versioned_name = CLASS_GetVersionedName( name ); > + > + classPtr = CLASS_FindClass( versioned_name, hInstance ); > + if (!classPtr) > { > - SetLastError( ERROR_CLASS_DOES_NOT_EXIST ); > - return FALSE; > + /* > + * FIXME: It's not correct to fall back to the normal class, but our > + * comctl32 implementation depends on it... > + */ > + classPtr = CLASS_FindClass( name, hInstance ); > + if (!classPtr) > + { > + SetLastError( ERROR_CLASS_DOES_NOT_EXIST ); > + return FALSE; > + } > } > wc->style = classPtr->style; > wc->lpfnWndProc = WINPROC_GetProc( classPtr->winproc, TRUE ); GetClassInfo part looks okay. It shouldn't break anything even if we had it today, except that it adds bogus context lookup that is a waste of time. > diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h > index 0b5b2ac..0c212ce 100644 > --- a/dlls/user32/user_private.h > +++ b/dlls/user32/user_private.h > @@ -281,6 +281,8 @@ extern void SPY_EnterMessage( INT iFlag, HWND hwnd, UINT msg, WPARAM wParam, LPA > extern void SPY_ExitMessage( INT iFlag, HWND hwnd, UINT msg, > LRESULT lReturn, WPARAM wParam, LPARAM lParam ) DECLSPEC_HIDDEN; > > +extern LPCWSTR CLASS_GetVersionedName( LPCWSTR name ) DECLSPEC_HIDDEN; > + > #include "pshpack1.h" > > typedef struct > diff --git a/dlls/user32/win.c b/dlls/user32/win.c > index a7be4a3..42676b5 100644 > --- a/dlls/user32/win.c > +++ b/dlls/user32/win.c > @@ -1740,9 +1740,11 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExA( DWORD exStyle, LPCSTR className, > if (!IS_INTRESOURCE(className)) > { > WCHAR bufferW[256]; > + LPCWSTR versioned_name; > if (!MultiByteToWideChar( CP_ACP, 0, className, -1, bufferW, sizeof(bufferW)/sizeof(WCHAR) )) > return 0; > - return wow_handlers.create_window( (CREATESTRUCTW *)&cs, bufferW, instance, FALSE ); > + versioned_name = CLASS_GetVersionedName( bufferW ); > + return wow_handlers.create_window( (CREATESTRUCTW *)&cs, versioned_name, instance, FALSE ); > } > /* Note: we rely on the fact that CREATESTRUCTA and */ > /* CREATESTRUCTW have the same layout. */ > @@ -1760,6 +1762,7 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExW( DWORD exStyle, LPCWSTR className, > HINSTANCE instance, LPVOID data ) > { > CREATESTRUCTW cs; > + LPCWSTR versioned_name = CLASS_GetVersionedName( className ); > > cs.lpCreateParams = data; > cs.hInstance = instance; > @@ -1774,7 +1777,7 @@ HWND WINAPI DECLSPEC_HOTPATCH CreateWindowExW( DWORD exStyle, LPCWSTR className, > cs.lpszClass = className; > cs.dwExStyle = exStyle; > > - return wow_handlers.create_window( &cs, className, instance, TRUE ); > + return wow_handlers.create_window( &cs, versioned_name, instance, TRUE ); > } > Looks fine, but it could go to WIN_CreateWindowEx(). There's one potentially very important thing about all this, according to docs every winproc call is guaranteed to happen under correct context. So basically if you send a message while particular context was activated, if this window belongs to different thread docs imply that context is temporary activated around winproc call on that receiving thread. I was unable to make this happen, maybe a fresh pair of eyes will be helpful.

4 6

Re: [PATCH] d3dx10: Parially implement async data loaders interfaces
by Matteo Bruni Nov. 6, 2016

Nov. 6, 2016

2016-11-06 19:45 GMT+01:00 Nikolay Sivov <nsivov(a)codeweavers.com>: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d3dx10_43/async.c | 296 ++++++++++++++++++++++++++++++++++++++++-- > dlls/d3dx10_43/tests/d3dx10.c | 36 ++--- > 2 files changed, 306 insertions(+), 26 deletions(-) > > diff --git a/dlls/d3dx10_43/async.c b/dlls/d3dx10_43/async.c > index f49eb92..a9cbde5 100644 > --- a/dlls/d3dx10_43/async.c > +++ b/dlls/d3dx10_43/async.c > +/* Memory data loader. */ > +static HRESULT WINAPI memorydataloader_Load(ID3DX10DataLoader *loader) > +{ > + TRACE("loader %p\n", loader); We usually put a period at the end of trace messages. By the way, these comments don't add much IMO, it's pretty clear where each interface implementation starts and ends. > +static HRESULT WINAPI memorydataloader_Decompress(ID3DX10DataLoader *iface, void **data, SIZE_T *size) > +{ > + struct asyncdataloader *loader = impl_from_ID3DX10DataLoader(iface); > + > + FIXME("loader %p, data %p, size %p semi-stub\n", loader, data, size); > + > + *data = loader->data; > + *size = loader->size; > + > + return S_OK; > +} Why the fixme? What's still missing here? > +static HRESULT WINAPI filedataloader_Load(ID3DX10DataLoader *iface) > +{ > + struct asyncdataloader *loader = impl_from_ID3DX10DataLoader(iface); > + HANDLE mapping; > + > + TRACE("loader %p\n", loader); > + > + if (loader->u.file == INVALID_HANDLE_VALUE) > + return D3D10_ERROR_FILE_NOT_FOUND; > + > + if (loader->data) > + return S_OK; > + > + GetFileSize(loader->u.file, (DWORD*)&loader->size); > + mapping = CreateFileMappingW(loader->u.file, NULL, PAGE_READONLY, 0, 0, NULL); > + if (!mapping) > + return E_FAIL; > + > + loader->data = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, 0); > + CloseHandle(mapping); > + if (!loader->data) > + return E_FAIL; > + > + return S_OK; > +} Creating and keeping a file mapping open might not be correct. What happens on Windows if e.g. you try to delete the file after Load() but before Decompress()? The issue might also be between Decompress() and Destroy(). I guess I'd like some more tests for the file and the resource loaders... > + if (!(rsrc = FindResourceA(module, resource, (LPSTR)RT_RCDATA))) Please avoid LPtypes and casting away const, here and below.

2 2

Re: [PATCH] ntdll: Add a simple test for RtlDosApplyFileIsolationRedirection_Ustr
by Nikolay Sivov Nov. 6, 2016

Nov. 6, 2016

On 06.11.2016 4:54, Ivan Akulinchev wrote: > Signed-off-by: Ivan Akulinchev <ivan.akulinchev(a)gmail.com> > --- > dlls/ntdll/tests/Makefile.in | 1 + > dlls/ntdll/tests/sxs.c | 141 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 142 insertions(+) > create mode 100644 dlls/ntdll/tests/sxs.c > > diff --git a/dlls/ntdll/tests/Makefile.in b/dlls/ntdll/tests/Makefile.in > index fc352dd..7fe1a70 100644 > --- a/dlls/ntdll/tests/Makefile.in > +++ b/dlls/ntdll/tests/Makefile.in > @@ -21,5 +21,6 @@ C_SRCS = \ > rtlbitmap.c \ > rtlstr.c \ > string.c \ > + sxs.c \ > threadpool.c \ > time.c > diff --git a/dlls/ntdll/tests/sxs.c b/dlls/ntdll/tests/sxs.c > new file mode 100644 > index 0000000..e04803b > --- /dev/null > +++ b/dlls/ntdll/tests/sxs.c > @@ -0,0 +1,141 @@ > +/* > + * Tests for the Windows side-by-side assemblies. > + * > + * Copyright 2016 Ivan Akulinchev > + * > + * 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 "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windef.h" > +#include "winbase.h" > +#include "winuser.h" > +#include "winnls.h" > +#include "winternl.h" > + > +#include "wine/test.h" > + > +static NTSTATUS (WINAPI *pRtlDosApplyFileIsolationRedirection_Ustr)(ULONG, > + PUNICODE_STRING, > + PUNICODE_STRING, > + PUNICODE_STRING, > + PUNICODE_STRING, > + PUNICODE_STRING *, > + PULONG, > + PULONG, > + PULONG); > + > +static const CHAR manifest_name[] = "winsxs.manifest"; > + > +/* Note: '\n' is intentionally skipped. */ > +static const CHAR manifest[] = > + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>" > + "<assembly xmlns=\"urn:schemas-microsoft-com:asm.v1\" manifestVersion=\"1.0\">" > + " <assemblyIdentity type=\"win32\"" > + " name=\"Wine.NtDll.Tests\"" > + " version=\"1.0.0.0\"" > + " processorArchitecture=\"*\" />" > + " <dependency>" > + " <dependentAssembly>" > + " <assemblyIdentity type=\"win32\"" > + " name=\"microsoft.windows.common-controls\"" > + " version=\"6.0.0.0\"" > + " processorArchitecture=\"*\"" > + " publicKeyToken=\"6595b64144ccf1df\"" > + " language=\"*\" />" > + " </dependentAssembly>" > + " </dependency>" > + "</assembly>"; > + > +static void test_rtl_dos_apply_file_isolation_redirect(void) > +{ > + NTSTATUS status; > + WCHAR buffer[MAX_PATH]; > + UNICODE_STRING original_name; > + UNICODE_STRING extension; > + UNICODE_STRING unknown1; > + UNICODE_STRING unknown2; > + UNICODE_STRING *path; > + > + static const WCHAR original_nameW[] = {'c','o','m','c','t','l','3','2',0}; > + static const WCHAR extensionW[] = {'.','d','l','l',0}; > + static const WCHAR emptyW[] = {'\0',0}; > + > + RtlInitUnicodeString(&original_name, original_nameW); > + RtlInitUnicodeString(&extension, extensionW); > + RtlInitUnicodeString(&unknown2, emptyW); > + > + unknown1.Buffer = buffer; > + unknown1.Length = 0; > + unknown1.MaximumLength = MAX_PATH; > + > + status = pRtlDosApplyFileIsolationRedirection_Ustr(1, &original_name, &extension, &unknown1, > + &unknown2, &path, NULL, NULL, NULL); Where does this prototype come from and why do we need this function in a first place?

2 2

Re: [PATCH] vbscript: Call GetItemInfo() with original item name
by Marvin Nov. 6, 2016

Nov. 6, 2016

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=26469 Your paranoid android. === w1064 (64 bit) === run.c:190: Test failed: test.JS: L"WScript.FullName = " run.c:190: Test failed: test.JS: L"WScript.Path = "

1 0

Re: [PATCH 2/2] crypt32: importing system root certs into volatile registry keys instead of dedicated RootStore
by Jacek Caban Nov. 4, 2016

Nov. 4, 2016

Hi Donat, The patch looks mostly good, I have just two final comments. On 10/28/16 5:16 PM, Donat Enikeev wrote: > -WINECRYPT_CERTSTORE *CRYPT_RootOpenStore(HCRYPTPROV hCryptProv, DWORD dwFlags) > +void CRYPT_ImportSystemRootCertsToReg(void) > { > - TRACE("(%ld, %08x)\n", hCryptProv, dwFlags); > + HCERTSTORE store = NULL; > + HKEY key; > + LONG rc; > + HANDLE mutex; > > - if (dwFlags & CERT_STORE_DELETE_FLAG) > + TRACE("\n"); > + > + mutex = CreateMutexW( NULL, FALSE, mutex_nameW); > + if (!mutex) There is a race in here. Sorry about wrong suggestion earlier, CreateSemaphore seems more appropriate here. If semaphore is already created, we need to wait for it to be satisfied. Otherwise we could try to read partially processed registries. > { > - WARN("root store can't be deleted\n"); > - SetLastError(ERROR_ACCESS_DENIED); > - return NULL; > + ERR("Failed to create mutex, %08x\n", GetLastError()); > + return; > } > - if (!CRYPT_rootStore) > + if ( GetLastError() == ERROR_ALREADY_EXISTS ) > + return; > + > + if (!(store = create_root_store())) > + return; > + > + rc = RegCreateKeyExW(HKEY_LOCAL_MACHINE, certs_root_pathW, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &key, 0); > + if (!rc) > { > - HCERTSTORE root = create_root_store(); > + if (!CRYPT_SerializeContextsToReg(key, REG_OPTION_VOLATILE, pCertInterface, store)) > + WARN("Failed to import system certs into registry, %08x\n", GetLastError()); > > - InterlockedCompareExchangePointer((PVOID *)&CRYPT_rootStore, root, > - NULL); > - if (CRYPT_rootStore != root) > - CertCloseStore(root, 0); > + RegCloseKey(key); > } > - CRYPT_rootStore->vtbl->addref(CRYPT_rootStore); > - return CRYPT_rootStore; > -} > > -void root_store_free(void) > -{ > - CertCloseStore(CRYPT_rootStore, 0); > -} > + CertCloseStore(store, 0); > +} > \ No newline at end of file > diff --git a/dlls/crypt32/store.c b/dlls/crypt32/store.c > index 356712b..19eca22 100644 > --- a/dlls/crypt32/store.c > +++ b/dlls/crypt32/store.c > @@ -424,21 +424,15 @@ static WINECRYPT_CERTSTORE *CRYPT_SysRegOpenStoreW(HCRYPTPROV hCryptProv, > SetLastError(E_INVALIDARG); > return NULL; > } > - /* FIXME: In Windows, the root store (even the current user location) is > - * protected: adding to it or removing from it present a user interface, > - * and the keys are owned by the system process, not the current user. > - * Wine's registry doesn't implement access controls, so a similar > - * mechanism isn't possible yet. > - */ > - if ((dwFlags & CERT_SYSTEM_STORE_LOCATION_MASK) == > - CERT_SYSTEM_STORE_LOCAL_MACHINE && !lstrcmpiW(storeName, rootW)) > - return CRYPT_RootOpenStore(hCryptProv, dwFlags); > > switch (dwFlags & CERT_SYSTEM_STORE_LOCATION_MASK) > { > case CERT_SYSTEM_STORE_LOCAL_MACHINE: > root = HKEY_LOCAL_MACHINE; > base = CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH; > + /* If the HKLM\Root certs are requested, expressing system certs into the registry */ > + if (!lstrcmpiW(storeName, rootW)) > + CRYPT_ImportSystemRootCertsToReg(); I'm a bit nervous about implications of this patch. This will allow changing the store, which was not possible earlier. Let me explain. If user has sufficient rights: - On Windows: warning dialogs appear when application tries to modify root store. It's allowed, but only with user confirmation (which makes me think it's not a common way for installing new certs). - On current Wine: it's not allowed. - With your patch: it's allowed. Ideally we'd do what Windows does, but that's a separated issue. I'd propose to change the patch to simply behave like current Wine does in the regards. It should be as simple as: dwFlags |= CERT_STORE_READONLY_FLAG; Does that sounds good to you? Thanks, Jacek

2 5

Re: msvcirt: Avoid "array subscript above array bounds" warnings in test_ostrstream and test_strstreambuf.
by Piotr Caban Nov. 4, 2016

Nov. 4, 2016

Hi, sorry for long reply time. On 10/30/16 14:55, Gerald Pfeifer wrote: > - ok(ssb1.base.ebuf == buffer + 0x7fffffff || ssb1.base.ebuf == (char*)-1, > + ok((size_t)ssb1.base.ebuf == (size_t)buffer + 0x7fffffff || ssb1.base.ebuf == (char*)-1, Is casting buffer to char* enough to get away with the warning? I think I would prefer it to be done this way if it works. I'm also not sure if this warning is useful. I don't see anything wrong in computing the pointer as long as it's not accessed. I guess it can't be disabled without also disabling some useful warnings? Thanks, Piotr

1 0

Re: [PATCH v2 2/7] winebus.sys: Implement removing IOHID devices
by Sebastian Lackner Nov. 4, 2016

Nov. 4, 2016

2016-11-04 13:45 GMT+01:00 Aric Stewart <aric(a)codeweavers.com>: > > v2: Suggestions from Ken Thomases > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_iohid.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > I haven't reviewed the whole series yet, but the result of subtracting two ULONG_PTRs might not fit in an int return value.

1 0

Re: [v2 PATCH] vbscript: Implement StrComp()
by Marvin Nov. 4, 2016

Nov. 4, 2016

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=26449 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

Re: [PATCH] vbscript: Implement StrComp()
by Dmitry Timoshkov Nov. 4, 2016

Nov. 4, 2016

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + TRACE("(%s %s ...)\n", debugstr_variant(args), debugstr_variant(args+1)); > + > + assert(args_cnt == 2 || args_cnt == 3); > + > + if(V_VT(args) != VT_BSTR || V_VT(args+1) != VT_BSTR) { > + FIXME("args[0] = %s, args[1] = %s\n", debugstr_variant(args), debugstr_variant(args+1)); > + return E_NOTIMPL; > + } > + > + if (args_cnt == 3) { > + hres = to_int(args+2, &mode); > + if(FAILED(hres)) > + return hres; > + > + if (mode != 0 && mode != 1) { > + FIXME("unknown compare mode = %d\n", mode); > + return E_FAIL; > + } > + } > + else > + mode = 0; > + > + left = V_BSTR(args); > + right = V_BSTR(args+1); Is it possible to consistently use single way of accessing the args array, for instance the indexed args[i] form? -- Dmitry.

2 2

Re: [PATCH] vbscript: Implement StrComp()
by Jacek Caban Nov. 4, 2016

Nov. 4, 2016

Hi Nikolay, The patch looks mostly good. On 04.11.2016 07:34, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/vbscript/global.c | 43 ++++++++++++++++++++++++++++++++++++++++--- > dlls/vbscript/tests/api.vbs | 15 +++++++++++++++ > 2 files changed, 55 insertions(+), 3 deletions(-) > > diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c > index 129d6ce..a872838 100644 > --- a/dlls/vbscript/global.c > +++ b/dlls/vbscript/global.c > @@ -1027,10 +1027,47 @@ static HRESULT Global_MidB(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARI > return E_NOTIMPL; > } > > -static HRESULT Global_StrComp(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res) > +static HRESULT Global_StrComp(vbdisp_t *This, VARIANT *args, unsigned args_cnt, VARIANT *res) > { > - FIXME("\n"); > - return E_NOTIMPL; > + BSTR left, right; > + int mode, ret; > + HRESULT hres; > + > + TRACE("(%s %s ...)\n", debugstr_variant(args), debugstr_variant(args+1)); > + > + assert(args_cnt == 2 || args_cnt == 3); > + > + if(V_VT(args) != VT_BSTR || V_VT(args+1) != VT_BSTR) { > + FIXME("args[0] = %s, args[1] = %s\n", debugstr_variant(args), debugstr_variant(args+1)); > + return E_NOTIMPL; > + } > + > + if (args_cnt == 3) { > + hres = to_int(args+2, &mode); A test where mode arg is of other type would be nice. Something like "1" (passed as a string). > + if(FAILED(hres)) > + return hres; > + > + if (mode != 0 && mode != 1) { > + FIXME("unknown compare mode = %d\n", mode); > + return E_FAIL; > + } > + } > + else > + mode = 0; > + > + left = V_BSTR(args); > + right = V_BSTR(args+1); > + > + V_VT(res) = VT_I2; > + ret = mode ? strcmpiW(left, right) : strcmpW(left, right); > + if (ret < 0) > + V_I2(res) = -1; > + else if (ret > 0) > + V_I2(res) = 1; > + else > + V_I2(res) = 0; Please use return_short() helper here. Thanks, Jacek

1 0

Re: [PATCH] wscript: Do case insensitive check for WSH/WScript item names
by Jacek Caban Nov. 4, 2016

Nov. 4, 2016

On 04.11.2016 06:36, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > > For https://bugs.winehq.org/show_bug.cgi?id=41651 > > programs/wscript/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/programs/wscript/main.c b/programs/wscript/main.c > index 3537f9d..e4bd676 100644 > --- a/programs/wscript/main.c > +++ b/programs/wscript/main.c > @@ -87,7 +87,7 @@ static HRESULT WINAPI ActiveScriptSite_GetItemInfo(IActiveScriptSite *iface, > { > WINE_TRACE("(%s %x %p %p)\n", wine_dbgstr_w(pstrName), dwReturnMask, ppunkItem, ppti); > > - if(strcmpW(pstrName, wshW) && strcmpW(pstrName, wscriptW)) > + if(strcmpiW(pstrName, wshW) && strcmpiW(pstrName, wscriptW)) > return E_FAIL; > > if(dwReturnMask & SCRIPTINFO_ITYPEINFO) { Looks good, but please add a test case. Thanks, Jacek

1 0

Re: [PATCH 1/3] dssenh/tests: The default container may already exist.
by Marvin Nov. 4, 2016

Nov. 4, 2016

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=26439 Your paranoid android. === w864 (32 bit dssenh) === dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. === w1064 (32 bit dssenh) === dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. === w1064 (64 bit dssenh) === dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:642: Test failed: Incorrect decrypted data. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical. dssenh.c:1348: Test failed: Decrypted data is not identical.

2 1

Re: [PATCH 2/3] dssenh/tests: RC4 has been crippled in recent Windows versions.
by Marvin Nov. 4, 2016

Nov. 4, 2016

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=26440 Your paranoid android. === w864 (32 bit dssenh) === dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. === w1064 (32 bit dssenh) === dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. === w1064 (64 bit dssenh) === dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical. dssenh.c:1349: Test failed: Decrypted data is not identical.

1 0

Re: [PATCH] vbscript: Implement StrComp()
by Marvin Nov. 4, 2016

Nov. 4, 2016

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=26432 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

Re: [PATCH (resend) 3/8] winebus.sys: Implement removing IOHID devices
by Ken Thomases Nov. 4, 2016

Nov. 4, 2016

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 | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c > index f3e282c..c25a3b8 100644 > --- a/dlls/winebus.sys/bus_iohid.c > +++ b/dlls/winebus.sys/bus_iohid.c > @@ -120,7 +120,9 @@ static DWORD CFNumberToDWORD(CFNumberRef num) > > static int compare_platform_device(DEVICE_OBJECT *device, void *platform_dev) > { > - return 0; > + IOHIDDeviceRef dev1 = *(IOHIDDeviceRef*)get_platform_private(device); > + IOHIDDeviceRef dev2 = (IOHIDDeviceRef)platform_dev; > + return dev1 == dev2; The return value here has the reverse sense from how the caller interprets it. The caller, bus_find_hid_device(), interprets it like the result from strcmp() or memcmp(). A value of 0 means equal. Here, you're returning true (1) when the devices are equal. > }

1 1

Re: [PATCH (resend) 5/8] winebus.sys: IOCTL_HID_GET_STRING for iohid
by Ken Thomases Nov. 4, 2016

Nov. 4, 2016

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 Nov. 4, 2016

Nov. 4, 2016

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 Nov. 4, 2016

Nov. 4, 2016

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 Nov. 3, 2016

Nov. 3, 2016

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 Nov. 3, 2016

Nov. 3, 2016

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 Nov. 3, 2016

Nov. 3, 2016

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 Nov. 3, 2016

Nov. 3, 2016

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 Nov. 2, 2016

Nov. 2, 2016

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 Nov. 2, 2016

Nov. 2, 2016

> +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 Nov. 2, 2016

Nov. 2, 2016

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

Re: [PATCH 2/2] gdiplus: Implement GdipBitmapGetHistogram()
by Vincent Povirk Nov. 1, 2016

Nov. 1, 2016

> + ok(ch1[0x3f] == WIDTH, "Got green (0x3f) %u\n", ch1[0x3f]); > + ok(ch2[0x20] == WIDTH, "Got blue (0x20) %u\n", ch1[0x20]); > + > +return; > + > + /* ARGB histogram from RGB data. */ > + stat = pGdipBitmapGetHistogram(bm, HistogramFormatARGB, 256, ch0, ch1, ch2, NULL); > + expect(InvalidParameter, stat); That return doesn't look like it's supposed to be there.

2 1

Re: [PATCH] ws2_32: Fix the default behavior of IPV6_V6ONLY
by Marvin Nov. 1, 2016

Nov. 1, 2016

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=26345 Your paranoid android. === w2008s64 (64 bit sock) === sock: unhandled exception c0000005 at 000007FEFF126F94

3 2

Re: [PATCH 2/2] gdiplus: Implement GdipBitmapGetHistogram()
by Dmitry Timoshkov Nov. 1, 2016

Nov. 1, 2016

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + for (x = 0; x < width; x++) > + for (y = 0; y < height; y++) > + { > + ARGB color; > + > + GdipBitmapGetPixel(bitmap, x, y, &color); > + set_histogram_point[format](color, ch0, ch1, ch2, ch3); > + } > + > + return Ok; > +} One of the very least things that could be done about this very ineffecient implementation is to make the width traversing an inner loop. -- Dmitry.

1 0

Re: wined3d: Add ARB_texture_env_crossbar extension.
by Henri Verbeet Nov. 1, 2016

Nov. 1, 2016

This isn't very useful on its own. Do you intend to use the extension for something?

1 0