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

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