August 2017 - wine-devel

Error handling in RpcBindingVectorFree (dlls/rpcrt4/rpc_binding.c)
by Gerald Pfeifer 22 Nov '20

22 Nov '20

Looking at RPC_STATUS WINAPI RpcBindingVectorFree( RPC_BINDING_VECTOR** BindingVector ) { RPC_STATUS status; ULONG c; TRACE("(%p)\n", BindingVector); for (c=0; c<(*BindingVector)->Count; c++) { status = RpcBindingFree(&(*BindingVector)->BindingH[c]); } HeapFree(GetProcessHeap(), 0, *BindingVector); *BindingVector = NULL; return RPC_S_OK; } we currently always ignore the outcome of RpcBindingFree and return RPC_S_OK. However, there is one case where RpcBindingFree returns something different (which is if *Binding is null when RPC_S_INVALID_BINDING is returned). What is the proper way of handling this? Just keeping the code as is and removing the unused status variable? Breaking the loop once RpcBindingFree returns something different from RPC_S_OK? Continuing and returning the first / the last status different from RPC_S_OK? Gerald

1 1

ntoskrnl.exe: unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces
by Qian Hong 28 Sep '18

28 Sep '18

Dear all, While test another online bank with wine ActiveX, I got an unimplemented fuction of ntoskrnl: IoGetDeviceInterfaces, I found it listed in http://source.winehq.org/WineAPI/ntoskrnl.html as a stup, so I can't understand this log: wine: Unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces called at address 0x7b839552 (thread 0022), starting debugger... Grateful for any explain! env: wine1.3.12 on Ubuntu 10.04 Here are the steps: 1. install an ActiveX from https://e.bank.ecitic.com/perbank5/plugs/CNCBSecPkg_EN.exe $ rm -rf ~/.wine $ winetricks -q mfc42 $ wine CNCBSecPkg_EN.exe fixme:ole:DllRegisterServer stub fixme:win:DisableProcessWindowsGhosting : stub fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:ole:CoCreateInstance no instance created for interface {ea1afb91-9e28-4b86-90e9-9e9f8a5eefaf} of class {56fdf344-fd6d-11d0-958a-006097c9a090}, hres is 0x80004002 fixme:sfc:SfcIsFileProtected ((nil), L"C:\\Program Files\\\4e2d\4fe1\94f6\884c\7f51\94f6\5b89\5168\63a7\4ef6\\unins000.exe") stub fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 0: stub! fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 -1: stub! fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 0: stub! wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting wine: Unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces called at address 0x7b839552 (thread 002b), starting debugger... wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting 2. open the online bank entry with wine builtin IE, then IE will crash: $ wine iexplore https://e.bank.ecitic.com/perbank5/signIn.do Please checkout the full log here: http://pastebin.com/rbAg7gwj Should I file a singel bug in ntoskrnl component , or separate bugs, one for ntoskrnl and one for the IE crashing? Generalliy what component should I switch while file a bug about IE crashing? Many thanks! -- Regards, Qian Hong - Sent from Ubuntu http://www.ubuntu.com/

5 6

COM tracing ...
by Michael Meeks 21 Aug '18

21 Aug '18

Hi guys, Rather a peripheral question; apologies for that - but I'd imagine there are experts here that may be able to help. I have a black-box problem - a Windows app dealing with confidential data that I can't easily touch (and thus can't get to run under Wine) - which does some small but critical automation of MS Office - using VBS / COM scripting. I'd -really- love some input on how best to trace that series of COM method calls on Windows itself ie. the (remote) service activation, and the RPC beyond that - particularly the method names, parameters etc. of the COM/OLE Office API. I've tried (so far): http://www.rohitab.com/apimonitor which, while closed, looks interesting; but traces a lot in a hard-to-search way and doesn't appear to do the trick. https://support.microsoft.com/en-us/help/926098/how-to-enable-com-and-com-d… Sounds useful: using Event Tracing for Windows (ETW) but has this rather unhelpful property: "The binary file must be formatted by Microsoft so that it can be analyzed. Please forward the .etl files to your support contact. ..." ;-) Component models used to be all the rage in my youth ;-) surely someone solved the "strace for COM calls" problem elegantly some-when ! Crazily - might it be possible to instrument, interpose and use Wine's COM impl. on Windows [ which sounds a bit 'exciting' ;-], or ? Anyhow - help much appreciated & sorry for the noise ! Regards, Michael. -- michael.meeks(a)collabora.com <><, Pseudo Engineer, itinerant idiot

2 4

ODBC Databases: Jet/Access success
by Barry Bird 14 Jun '18

14 Jun '18

Good Afternoon. In section 3.3.6.2 of your User Guide you ask readers to report successes with databases other than MS SQL. Well here's one (I know Access 2000 can work with Wine, but this doesn't require Access): How to set up Wine to enable Windows programs that read and write to Jet (Access) databases using ODBC. I write such programs in C and use the API defined in ODBC API Reference http://msdn.microsoft.com/en-us/library/ms714562%28VS.85%29.aspx They work fine on Windows and only require Mingw to be installed to compile and link them, both for console programs and those using Windows SDK. You then can write interactive programs in C which use a full-featured SQL database which comes bundled with Windows. No need to purchase Access. With the set-up below they will also run on Linux (Kubuntu 9.04) and Wine 1.1.26. You need to update the registry to install the Access drivers. Under Windows, export from the Registry to *.reg files the registry entries - HKLM\Software\ODBC - HKLM\Software\Microsoft\Jet all subsidiary keys and values come with them. You can carefully edit these files to remove drivers and DSNs you don't need. Import these files using the registry editor in wine: wine regedit.exe. You need to bring across from Windows to Windows\System32 under wine: clbcatq.dll comres.dll expsrv.dll msjet40.dll mswstr10.dll msjter40.dll msjint40.dll msjtes40.dll msrd3x40.dll odbc32.dll odbccp32.dll odbcji32.dll odbcjt32.dll odbcad32.exe odbcint.dll odbctrac.dll vbajet32.dll Register this server in wine: wine regsvr32.exe msjtes40.dll No need to install MDAC. To use Windows ODBC drivers, you have to override Wine's odbccp32.dll and odbc32.dll with the native versions because the Wine versions are currently wired directly to Linux's unixodbc. This can be done by setting up the ODBC Data Source Administrator odbcad32.exe using winecfg: - Add the program to the Applications tab - then in the libraries tab, pick from 'New override for library' drop-down odbc32.dll and odbccp32.dll add them and edit them to be Native for Windows. Then if you do wine odbcad32.exe this brings up the ODBC Data Source Administrator window as in the Windows Control Panel If needed, set up (System) DSNs using this program. Bring across the programs and *.mdb database files from Windows. Using winecfg, you need to set up each program you want to run with overrides for odbc32.dll and odbccp32.dll (Applications and Libraries tabs) as above. The programs should then run as they did in Windows but perhaps a bit slower with: wine Odbc-prog.exe This has been tested twice on a clean .wine install. I cannot vouch for support of all API and SQL facilities however. I hope someone finds this useful. Barry Bird

5 4

Re: [PATCH] arm64: Use __builtin_ms_va_list and __attribute__((ms_abi)) on arm64
by André Hentschel 13 Feb '18

13 Feb '18

Am 14.08.2017 um 12:35 schrieb Martin Storsjo: > Windows uses a different ABI for va_list on arm64 just like on x86_64. > > On x86_64, the calling convention for windows functions is completely > different from the one on other platforms. On arm64, they're mostly the > same, with the only exception being variadic functions (where all float > arguments are passed in integer registers, since the va_list is a single > pointer). > > Any functions using __builtin_ms_va_start need to be decorated with > __attribute__((ms_abi)). > > Add a check in configure for the support of __builtin_ms_va_list > (both features appared in clang at the same time, in the 5.0 release). > > This fixes running binaries that use e.g. printf style functions > (e.g. midl.exe from the windows 10 sdk), fixing > https://bugs.winehq.org/show_bug.cgi?id=38886. > > Signed-off-by: Martin Storsjo <martin(a)martin.st> > --- > clang 5.0, which this requires, isn't yet released, but is in RC2 and > will probably be released pretty soon. > --- > configure.ac | 7 +++++++ > include/msvcrt/crtdefs.h | 6 +++++- > include/windef.h | 6 +++++- > include/wine/test.h | 4 ++-- > 4 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 7d52b24..c33221f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -179,6 +179,13 @@ case $host in > CFLAGS="$CFLAGS -marm" > AC_SUBST(TARGETFLAGS,"-marm") > ;; > + aarch64*) > + AC_MSG_CHECKING([whether $CC supports __builtin_ms_va_list]) > + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdarg.h>]], [[void func(__builtin_ms_va_list *args);]])], > + [AC_MSG_RESULT([yes])], > + [AC_MSG_RESULT([no]) > + AC_MSG_ERROR([You need clang >= 5.0 to build Wine for arm64.])]) Hi, The upstreamed variadic function handling is terrific, thanks for that! I also like this patch as logical next step, I'm just a bit worried because it locks out current builds without upgrading to an RC... At the moment I'm doing winelib builds with gcc for Linux/ARM64 and clang builds for Android/ARM64. It's quite easy on Linux to pick up a new clang version, not sure how long it takes until clang5 is picked up in the Android NDK... On the other hand, there are not much people building Wine for ARM64, and those can pretty easily revert your patch until they are at clang5 OK... We need this improvement, I'll sign it off :) Now, we only need to convince distributions to avoid x18

3 4

Re: [PATCH] server_ioctl_file() - Prefix printed hex values with '0x'
by Alexandre Julliard 12 Oct '17

12 Oct '17

Chris Morgan <chmorgan(a)gmail.com> writes: > Avoids the ambiguity with non-alphanumeric hex values. > > Signed-off-by: Chris Morgan <chmorgan(a)gmail.com> > --- > dlls/ntdll/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c > index fd7f3dd..4d081b7 100644 > --- a/dlls/ntdll/file.c > +++ b/dlls/ntdll/file.c > @@ -1567,7 +1567,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle, HANDLE event, > SERVER_END_REQ; > > if (status == STATUS_NOT_SUPPORTED) > - FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n", > + FIXME("Unsupported ioctl 0x%x (device=0x%x access=0x%x func=0x%x method=0x%x)\n", That's mostly a matter of taste, so I don't think it's necessary to change it. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine Swag Sticker Poll
by Aric Stewart 10 Oct '17

10 Oct '17

Hello! So all the way back at wineconf I volunteered to be swag master for winehq. The idea is to have something that can be given away at trade shows, presentations, LUG visits and the like. Just something to help promote the visibility of the project. Jana here, CodeWeavers marketing director, has put together a sampling of designs to do as our first sticker and so now we are looking for some feedback on what people prefer. https://docs.google.com/forms/d/e/1FAIpQLSd5X_fXe9KSFAppRHz8kUo3XKeODyH5KeX… Rosanne? Could you direct me as to where this should be put up for the users as well? I am unfamiliar with how to communicate to our user base. The poll will run for as long as it feels like it needs to get some sort of winner. Comments and suggestions are always welcome! -aric

7 10

Re: [PATCH 1/2] comctl32/taskdialog: Initial support for callback procedure
by Fabian Maurer 12 Sep '17

12 Sep '17

Hi, Sorry, but there's a bunch of issues I need to point out. Of course we could address them later, but it'll mean rewriting a bunch of code, and I'd like to have it correct from the beginning. In general, I think this implementation limits us too much. For example, I don't think it's a good idea to have the TASKDIALOGCONFIG structure being filled in run_test_, we need to be able to control it easily. Second, we also need to be able to return different values in the callback, the current approach doesn't account for that. And the most important part, the separation from messages to send and received messages to test against. It's better to send the next message only when the last message arrived, to make sure we process them in the correct order. I attached a patch to show the problem (apply on top of my in-review patch). When sending all messages together, the first message is skipped, and we don't want that to happen. Please reconsider my approach, I can try to simplify it if you want, but I think the complexity is justified to solve a complicated problem. Regards, Fabian Maurer

2 3

Re: [v2 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 01 Sep '17

01 Sep '17

2017-08-31 20:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + if (!param->object_id || base->objects[param->object_id].creation_failed) It should be possible to replace this with a check for !*(void **)param->data. Unless you mean to use the flag for something else later on?

2 1

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangle geometry
by Henri Verbeet 01 Sep '17

01 Sep '17

On 31 August 2017 at 14:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > - if (transform) > - { > - if (!d2d_matrix_invert(&g_i, transform)) > - return D2DERR_UNSUPPORTED_OPERATION; > - d2d_point_transform(&point, &g_i, point.x, point.y); > - } > + if (FAILED(hr = geometry_inv_transform_point(transform, &point))) > + return hr; That's fine in principle, but please do it in a separate commit, and please follow the existing conventions. E.g.: if (transform && FAILED(hr = d2d_point_transform_inverse(&point, transform, point.x, point.y))) return hr; > + stroke_width /= 2.0f; > + > + rect = geometry->u.rectangle.rect; > + rect.left -= stroke_width; > + rect.right += stroke_width; > + rect.top -= stroke_width; > + rect.bottom += stroke_width; > + > + dx = max(fabsf((rect.right + rect.left) / 2.0f - point.x) - (rect.right - rect.left) / 2.0f, 0.0f); > + dy = max(fabsf((rect.bottom + rect.top) / 2.0f - point.y) - (rect.bottom - rect.top) / 2.0f, 0.0f); > + > + if ((*contains = tolerance * tolerance > (dx * dx + dy * dy))) > + { > + rect = geometry->u.rectangle.rect; > + stroke_width += tolerance; > + rect.left += stroke_width; > + rect.right -= stroke_width; > + rect.top += stroke_width; > + rect.bottom -= stroke_width; > + > + *contains = rect.left >= point.x || rect.right <= point.x || rect.top >= point.y || rect.bottom <= point.y; > + } I think that looks a little more complicated than it needs to be. How about: D2D1_POINT_2F d, s; ... s.x = rect.right - rect.left; s.y = rect.bottom - rect.top; d.x = fabsf((rect.right + rect.left) * 0.5f - point.x); d.y = fabsf((rect.bottom + rect.top) * 0.5f - point.y); if (d.x < (s.x - stroke_width) * 0.5f - tolerance && d.y < (s.y - stroke_width) * 0.5f - tolerance) { *contains = FALSE; return S_OK; } d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); *contains = (d.x * d.x + d.y * d.y) < tolerance * tolerance; I notice you don't have any tests with transformations, is that intentional? I think you'd need to transform the stroke width with the transformation matrix.

2 1

Re: [PATCH 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); Do you know what else is missing for ValidateTechnique()? Replying to myself: the return values mentioned in https://msdn.microsoft.com/en-us/library/windows/desktop/bb205835(v=vs.85).… give some ideas (although E_FAIL is not in the list, figures...)

2 1

Re: [PATCH 1/3] d3dx9/tests: Add test for effect with unsupported shader.
by Matteo Bruni 31 Aug '17

31 Aug '17

2017-08-31 19:40 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 185 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 185 insertions(+) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 9658d2dc82..3e8df11d24 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -6981,6 +6981,190 @@ static void test_effect_skip_constants(IDirect3DDevice9 *device) > effect->lpVtbl->Release(effect); > } > > +#if 0 > +vertexshader vs_arr[2] = > +{ > + asm > + { > + vs_3_0 > + def c0, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c0 > + }, > + > + asm > + { > + vs_3_sw > + def c256, 1, 1, 1, 1 > + dcl_position o0 > + mov o0, c256 > + }, > +}; > + > +int i; > + > +technique tech0 > +{ > + pass p0 > + { > + VertexShader = vs_arr[1]; > + } > +} > +technique tech1 > +{ > + pass p0 > + { > + VertexShader = vs_arr[i]; > + } > +} > +#endif > +static const DWORD test_effect_unsupported_shader_blob[] = > +{ > + 0xfeff0901, 0x000000ac, 0x00000000, 0x00000010, 0x00000004, 0x00000020, 0x00000000, 0x00000002, > + 0x00000001, 0x00000002, 0x00000007, 0x615f7376, 0x00007272, 0x00000002, 0x00000000, 0x0000004c, > + 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x00000000, 0x00000002, 0x00000069, 0x00000003, > + 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000003, 0x00003070, 0x00000006, > + 0x68636574, 0x00000030, 0x00000004, 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, > + 0x00000003, 0x00003070, 0x00000006, 0x68636574, 0x00000031, 0x00000002, 0x00000002, 0x00000006, > + 0x00000005, 0x00000004, 0x00000018, 0x00000000, 0x00000000, 0x0000002c, 0x00000048, 0x00000000, > + 0x00000000, 0x00000074, 0x00000000, 0x00000001, 0x0000006c, 0x00000000, 0x00000001, 0x00000092, > + 0x00000000, 0x00000058, 0x00000054, 0x000000a0, 0x00000000, 0x00000001, 0x00000098, 0x00000000, > + 0x00000001, 0x00000092, 0x00000000, 0x00000084, 0x00000080, 0x00000002, 0x00000002, 0x00000001, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0000, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40000, 0x0000ffff, 0x00000002, > + 0x00000038, 0xfffe0300, 0x05000051, 0xa00f0100, 0x3f800000, 0x3f800000, 0x3f800000, 0x3f800000, > + 0x0200001f, 0x80000000, 0xe00f0000, 0x02000001, 0xe00f0000, 0xa0e40100, 0x0000ffff, 0x00000001, > + 0x00000000, 0xffffffff, 0x00000000, 0x00000002, 0x000000e4, 0x00000008, 0x615f7376, 0x00007272, > + 0x46580200, 0x0023fffe, 0x42415443, 0x0000001c, 0x00000057, 0x46580200, 0x00000001, 0x0000001c, > + 0x00000100, 0x00000054, 0x00000030, 0x00000002, 0x00000001, 0x00000034, 0x00000044, 0xabab0069, > + 0x00020000, 0x00010001, 0x00000001, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > + 0x4d007874, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, 0x6d6f4320, > + 0x656c6970, 0x2e392072, 0x392e3932, 0x332e3235, 0x00313131, 0x0002fffe, 0x54494c43, 0x00000000, > + 0x000cfffe, 0x434c5846, 0x00000001, 0x10000001, 0x00000001, 0x00000000, 0x00000002, 0x00000000, > + 0x00000000, 0x00000004, 0x00000000, 0xf0f0f0f0, 0x0f0f0f0f, 0x0000ffff, 0x00000000, 0x00000000, > + 0xffffffff, 0x00000000, 0x00000001, 0x0000000a, 0x615f7376, 0x315b7272, 0x0000005d, > +}; > + > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS 81 > +#define TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN 14 > + > +static void test_effect_unsupported_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DVertexShader9 *vshader; > + unsigned int passes_count; > + IDirect3DDevice9 *device; > + UINT byte_code_size; > + ID3DXEffect *effect; > + IDirect3D9 *d3d; > + void *byte_code; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + if (!(window = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, > + 640, 480, NULL, NULL, NULL, NULL))) > + { > + skip("Couldn't create application window\n"); > + return; > + } > + if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION))) > + { > + skip("Couldn't create IDirect3D9 object\n"); > + DestroyWindow(window); > + return; > + } > + present_parameters.Windowed = TRUE; > + present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD; > + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, > + D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device); > + if (FAILED(hr)) { > + skip("Failed to create IDirect3DDevice9 object, hr %#x\n", hr); > + IDirect3D9_Release(d3d); > + DestroyWindow(window); > + return; > + } > + > + hr = D3DXCreateEffectEx(device, test_effect_unsupported_shader_blob, sizeof(test_effect_unsupported_shader_blob), > + NULL, NULL, NULL, 0, NULL, &effect, NULL); > + todo_wine > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + if (FAILED(hr)) > + { > + skip("Failed to create effect, skipping test.\n"); > + goto cleanup; > + } > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == E_FAIL, "Got result %#x.\n", hr); > + effect->lpVtbl->SetInt(effect, "i", 0); > + hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech0"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!vshader, "Got non NULL vshader.\n"); > + > + hr = effect->lpVtbl->EndPass(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->End(effect); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect->lpVtbl->SetTechnique(effect, "tech1"); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->Begin(effect, &passes_count, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(!!vshader, "Got NULL vshader.\n"); > + hr = IDirect3DVertexShader9_GetFunction(vshader, NULL, &byte_code_size); > + ok(hr == D3D_OK, "Got result %x.\n", hr); > + byte_code = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, byte_code_size); > + hr = IDirect3DVertexShader9_GetFunction(vshader, byte_code, &byte_code_size); > + ok(hr == D3D_OK, "Got result %x.\n", hr); > + ok(byte_code_size == TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_LEN * sizeof(DWORD), > + "Got unexpected byte code size %u.\n", byte_code_size); > + ok(!memcmp(byte_code, > + &test_effect_unsupported_shader_blob[TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS], > + byte_code_size), "Incorrect shader selected.\n"); I was confused by this at first, I thought somehow the unsupported vs_3_sw shader was set. That's not the case, TEST_EFFECT_UNSUPPORTED_SHADER_VSHADER_POS is the position of the vs_3_0 shader, but the naming throw me off. Can you rename those defines to "BYTECODE_VS_3_0_POS" or something that makes it clear that it refers to the hardware shader in vs_arr[0]?

2 2

Re: [PATCH v2 10/11] progman: Use proper size for combobox controls
by Alexandre Julliard 31 Aug '17

31 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/progman/progman.rc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/programs/progman/progman.rc b/programs/progman/progman.rc > index 4804330aed..9f451a736b 100644 > --- a/programs/progman/progman.rc > +++ b/programs/progman/progman.rc > @@ -119,7 +119,7 @@ LTEXT "", PM_PROGRAM, 95, 5, 90, 15 > LTEXT "From group:", -1, 5, 13, 90, 15 > LTEXT "", PM_FROM_GROUP, 95, 13, 90, 15 > LTEXT "&To group:", PM_TO_GROUP_TXT, 5, 28, 140, 15 > -COMBOBOX PM_TO_GROUP, 5, 38, 140, 50, WS_TABSTOP | CBS_DROPDOWNLIST > +COMBOBOX PM_TO_GROUP, 5, 38, 140, 60, WS_TABSTOP | CBS_DROPDOWNLIST That kind of change doesn't seem necessary. We don't need all comboboxes to have the exact same height, only something reasonable. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine fails to compile in kernel32
by Fabian Maurer 30 Aug '17

30 Aug '17

Hello wine-devel, I'm running ArchLinux with gcc 7.2.0. Since commit e99811aee2dff88c41d6e2875401810a85325839 wine fails to build with > ../../../wine-git/dlls/kernel32/module.c: In function ‘load_library’: > ../../../wine-git/dlls/kernel32/module.c:987:44: error: initializer element is not constant > static const DWORD unsupported_flags = load_library_search_flags | Minimal test-program to show the error, just use something like "gcc main.c" to compile. ---------- static const int load_library_search_flags = 0; int main() { static const int unsupported_flags = load_library_search_flags; return 0; } ---------- Looks like an compiler error, but I'm not sure. Mind looking into that? Regards, Fabian Maurer

1 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Nikolay Sivov 30 Aug '17

30 Aug '17

On 29.08.2017 20:06, Fabian Maurer wrote: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > Interesting thing to test I think is to verify if current cross-built winecfg shows the same issue on Windows, before changing them all.

2 1

Re: [PATCH] winecfg: Use proper size for combobox controls
by Alexandre Julliard 30 Aug '17

30 Aug '17

Fabian Maurer <dark.shadow4(a)web.de> writes: > Fixes Bug 43632. > winecfg used the broken behavior that was fixed > in f7f7b89e2e9117811c91269643868c6d063db5e9, > so we need to adjust it. > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > programs/winecfg/winecfg.rc | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) There are resources in other modules that look like they would have the same problem, a grep over the codebase would probably be useful. -- Alexandre Julliard julliard(a)winehq.org

2 3

WINED3DFMT_RESZ in wined3d.
by lorenzo Skotos 30 Aug '17

30 Aug '17

Hi. I was investigating a regression that cause the CheckDeviceFormat function to return WINED3DERR_NOTAVAILABLE when quering for RESZ surfaces. (WINED3DFMT_RESZ). This is causing OblivionReloaded (that worked properly until wine 2.5) to not work properly (it detect an incompatible hardware in wine or NVIDIA in wine-staging). I tacked the regression to this commit : https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979… [https://camo.githubusercontent.com/db3db2bf09d082a1f4244859890e83954d8410ab…]<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> wined3d: Allow all formats with "glInternal" set in CheckSurfaceCapab… · wine-mirror/wine@8c98be4<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…> github.com …ility(). Rendertarget, depth/stencil and texturing restrictions are applied later in wined3d_check_device_format(), so to the extent this make a difference it mostly affects off-screen plain surf... I tried to fix this issue. The last patch I made in my local branch is adding a wined3d_format_texture_info in the format_texture_info array: + {WINED3DFMT_RESZ, GL_DEPTH24_STENCIL8, GL_DEPTH24_STENCIL8, 0, + GL_DEPTH_STENCIL, GL_UNSIGNED_INT, 0, + WINED3DFMT_FLAG_TEXTURE | WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL | WINED3DFMT_FLAG_RENDERTARGET | WINED3DFMT_FLAG_FBO_ATTACHABLE, + ARB_FRAMEBUFFER_OBJECT, NULL}, However I know almost nothing about the internal work of wined3d and opengl and don't know if this kind of addition is correct. OblivionReloaded works and can properly use the RESZ depth texture. I based this patch on the INTZ and RAWZ surfaces that are somewhat reported to be similar in scope in some docs. Also I don't know if the flags are correct or even meaningful in this scope. I could not find any documentation about the internal meaning of these flags. As the good Jon Snow I know nothing. Note I never did a patch in wine before (except adding some debug message). So please tell me if I'm in a completly wrong direction. Also note that I didn't run the test suite. Using make test is failing hard on an advapi32 test even without this patch applied. I attached the full patch file.

1 0

Re: [PATCH] user32: In GetMinMaxInfo, use the default X pos even if the Y pos changes
by Alexandre Julliard 30 Aug '17

30 Aug '17

Andrew Eikum <aeikum(a)codeweavers.com> writes: > This fixes maximizing to the wrong monitor on a Mac with multiple > monitors. Maximizing with multiple monitors continues to work correctly > on Linux for me. > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > dlls/user32/winpos.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) I'm not sure it makes sense to use only one coordinate of the monitor rectangle. What are the coordinate values that trigger this? -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/2] d3dx9: Get rid of constants modification bitmasks.
by Matteo Bruni 29 Aug '17

29 Aug '17

2017-08-10 12:02 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> The patch is fine overall, I only have a few questions / nitpicks. I'll skip the part about the delay with the review because it's old news by now... :/ > --- > dlls/d3dx9_36/d3dx9_private.h | 3 +- > dlls/d3dx9_36/preshader.c | 312 ++++++++++++++++++------------------------ > 2 files changed, 135 insertions(+), 180 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index 2727463a18..5ac5e63aeb 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -230,7 +230,7 @@ enum pres_reg_tables > struct d3dx_const_param_eval_output > { > struct d3dx_parameter *param; > - unsigned int table; > + enum pres_reg_tables table; The change is good but (unrelated to this patch) the enum type should really use the singular form. > static void dump_bytecode(void *data, unsigned int size) > @@ -674,6 +614,40 @@ static HRESULT append_const_set(struct d3dx_const_tab *const_tab, struct d3dx_co > return D3D_OK; > } > > +static void append_pres_const_set(struct d3dx_const_tab *const_tab, struct d3dx_preshader *pres) Nitpick: this function appends all the struct d3dx_const_param_eval_output from the preshader so calling it "append_pres_const_sets" (or even something more explicit to that effect, e.g. clarifying that you're appending entries about the preshader outputs, which are shader inputs) would be preferable. > +{ > + unsigned int i; > + struct d3dx_const_param_eval_output const_set; > + > + memset(&const_set, 0, sizeof(const_set)); You could initialize const_set in the definition instead, like: struct d3dx_const_param_eval_output const_set = {NULL}; which looks slightly better to me. It doesn't matter much of course. > @@ -971,10 +946,30 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab > if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > goto cleanup; > } > + if (pres) > + append_pres_const_set(out, pres); > if (out->const_set_count) > { > struct d3dx_const_param_eval_output *new_alloc; > > + qsort(out->const_set, out->const_set_count, sizeof(*out->const_set), compare_const_set); > + for (i = 0; i < out->const_set_count - 1; ++i) > + { > + if (out->const_set[i].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i + 1].constant_class == D3DXPC_FORCE_DWORD > + && out->const_set[i].table == out->const_set[i + 1].table > + && out->const_set[i].register_index + out->const_set[i].register_count > + >= out->const_set[i + 1].register_index) > + { > + out->const_set[i].register_count = out->const_set[i + 1].register_index > + + out->const_set[i + 1].register_count - out->const_set[i].register_index; I think this might lose account of some registers. E.g. consider the case where const_set[i] has register_index == 0 and register_count == 4 while const_set[i + 1] has register_index == 1 and register_count == 1. This would seem to lose track of the last two components. I know that preshaders found in the wild seem not to overwrite already written output registers but I wouldn't feel safe with (silently) depending on it. > + memmove(&out->const_set[i + 1], &out->const_set[i + 2], sizeof(out->const_set[i]) > + * (out->const_set_count - i - 2)); > + --out->const_set_count; > + --i; Mostly matter of personal preference but rather than decrementing the for counter in the if when coalescing I'd write this as a while and increment "i" only in the (currently absent) else case. > @@ -1529,22 +1534,56 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > data += param->rows * param->columns; > } > start_offset = get_offset_reg(table, const_set->register_index); > - if (table_info[table].type == param_type) > - regstore_set_modified(rs, table, start_offset, > - get_offset_reg(table, const_set->register_count) * const_set->element_count); > - else > + if (table_info[table].type != param_type) > regstore_set_data(rs, table, start_offset, (unsigned int *)rs->tables[table] + start_offset, > get_offset_reg(table, const_set->register_count) * const_set->element_count, param_type); > } > + > + > const_tab->update_version = new_update_version; One blank line too many. > -} > + if (!update_device) > + return D3D_OK; > > + for (const_idx = 0; const_idx < const_tab->const_set_count; ++const_idx) > + { > + struct d3dx_const_param_eval_output *const_set = &const_tab->const_set[const_idx]; > + > + if (device_update_all || (const_set->param > + ? is_param_dirty(const_set->param, update_version) : pres_dirty)) > + { > + enum pres_reg_tables table = const_set->table; > + > + if (table == device_table && device_start + device_count == const_set->register_index) Again nitpicking: are there better names for those variables? Maybe use the "current_" prefix instead? It's pretty clear what's going on either way, so up to you.

2 6

Re: [PATCH] comctl32/taskdialog: Add callback and tests
by Nikolay Sivov 28 Aug '17

28 Aug '17

On 28.07.2017 19:53, Fabian Maurer wrote: > > +static HRESULT callback(struct taskdialog_info *dialog_info, UINT uNotification, WPARAM wParam, LPARAM lParam) > +{ > + if(dialog_info->callback) > + return dialog_info->callback(dialog_info->hwnd, uNotification, wParam, lParam, dialog_info->callback_data); > + return S_OK; > +} Please use consistent naming and formatting, like space after 'if' for example, and uNotification -> notification. > + > static INT_PTR CALLBACK taskdialog_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) > { > + static const WCHAR taskdialog_info_propnameW[] = {'T','a','s','k','D','i','a','l','o','g','I','n','f','o',0}; > + struct taskdialog_info *dialog_info; > + > TRACE("hwnd=%p msg=0x%04x wparam=%lx lparam=%lx\n", hwnd, msg, wParam, lParam); > > + if(msg != WM_INITDIALOG && msg != WM_NCDESTROY) > + dialog_info = GetPropW(hwnd, taskdialog_info_propnameW); > + Why WM_NCDESTROY is a special case? > switch (msg) > { > + case WM_INITDIALOG: > + dialog_info = (struct taskdialog_info *)lParam; > + dialog_info->hwnd = hwnd; > + SetPropW(hwnd, taskdialog_info_propnameW, dialog_info); > + > + callback(dialog_info, TDN_DIALOG_CONSTRUCTED, 0, 0); > + callback(dialog_info, TDN_CREATED, 0, 0); > + break; This looks suspicious, documentation is not strict on this, but it seems TDN_CREATED corresponds for to WM_SHOWWINDOW stage. > case WM_COMMAND: > if (HIWORD(wParam) == BN_CLICKED) > { > WORD command_id = LOWORD(wParam); > - EndDialog(hwnd, command_id); > + click_button(dialog_info, command_id); > return TRUE; > } > break; > + case WM_DESTROY: > + callback(dialog_info, TDN_DESTROYED, 0, 0); > + RemovePropW(hwnd, taskdialog_info_propnameW); > + break; > + > + /* Custom messages*/ What do you mean by "custom"? > + > + case TDM_CLICK_BUTTON: > + click_button(dialog_info, wParam); > + break; > } > return FALSE; > } > @@ -507,17 +556,24 @@ HRESULT WINAPI TaskDialogIndirect(const TASKDIALOGCONFIG *taskconfig, int *butto > int *radio_button, BOOL *verification_flag_checked) > { > DLGTEMPLATE *template; > + struct taskdialog_info dialog_info; > INT ret; > > TRACE("%p, %p, %p, %p\n", taskconfig, button, radio_button, verification_flag_checked); > > + if(!taskconfig || taskconfig->cbSize != sizeof(TASKDIALOGCONFIG)) > + return E_INVALIDARG; Tests you added only check cbSize == 0 case. > + > + dialog_info.callback = taskconfig->pfCallback; > + dialog_info.callback_data = taskconfig->lpCallbackData; > + > template = create_taskdialog_template(taskconfig); > - ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, 0); > + ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, (LPARAM)&dialog_info); > Free(template); > > if (button) *button = ret; > if (radio_button) *radio_button = taskconfig->nDefaultButton; > - if (verification_flag_checked) *verification_flag_checked = TRUE; > + if (verification_flag_checked) *verification_flag_checked = FALSE; Why do you need this flag value change? > > return S_OK; > } > diff --git a/dlls/comctl32/tests/taskdialog.c b/dlls/comctl32/tests/taskdialog.c > index c937127897..867361617c 100644 > --- a/dlls/comctl32/tests/taskdialog.c > +++ b/dlls/comctl32/tests/taskdialog.c > @@ -24,13 +24,250 @@ > #include "winuser.h" > #include "commctrl.h" > > +#include "wine/list.h" > #include "wine/test.h" > #include "v6util.h" > +#include "msg.h" > + > +#define WM_TD_CALLBACK (WM_APP) /* Custom dummy message to wrap callback notifications */ > + > +#define NUM_MSG_SEQUENCES 1 > +#define TASKDIALOG_SEQ_INDEX 0 > + > +#define TEST_NUM_BUTTONS 20 /* Number of custom buttons to test with */ > + > +#define ID_START 20 /* Lower IDs might be used by the system */ > +#define ID_START_BUTTON (ID_START + 0) > > static HRESULT (WINAPI *pTaskDialogIndirect)(const TASKDIALOGCONFIG *, int *, int *, BOOL *); > static HRESULT (WINAPI *pTaskDialog)(HWND, HINSTANCE, const WCHAR *, const WCHAR *, const WCHAR *, > TASKDIALOG_COMMON_BUTTON_FLAGS, const WCHAR *, int *); > > +static struct msg_sequence *sequences[NUM_MSG_SEQUENCES]; > + > +/* Message lists to test against */ > + > +struct message_send_info > +{ > + UINT send_message; > + WPARAM send_wparam; > + LPARAM send_lparam; > + > + BOOL post; /* post instead of send */ > + const CHAR *title_target; /* control text, 0 means it's send to the dialog form instead */ > +}; > + > +struct message_info > +{ > + UINT recv_message; /* Message the callback receives */ > + WPARAM recv_wparam; > + LPARAM recv_lparam; > + > + HRESULT ret; /* Value the callback should return */ > + > + struct message_send_info send[9]; /* Message to send to trigger the next callback message */ > +}; > + > +static const struct message_info *current_message_info; > + > +static const struct message_info mes_return_press_ok[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_KEYDOWN, VK_RETURN, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_cancel_button_press[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { TDM_CLICK_BUTTON, IDOK, 0, TRUE }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, S_FALSE, {{ 0 }}}, > + { TDN_BUTTON_CLICKED, IDOK, 0, 0xFF, {{ 0 }}}, /* Random return value tested here */ > + { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; > + > +static const struct message_info mes_send_button_directly[] = { > + { TDN_CREATED, 0, 0, S_OK, { > + { WM_LBUTTONDOWN, MK_LBUTTON, 0, TRUE, "02" }, > + { WM_LBUTTONUP, 0, 0, TRUE, "02" }, > + { 0 }}}, > + { TDN_BUTTON_CLICKED, ID_START + 2, 0, S_OK, {{ 0 }}}, > + { 0 } > +}; This looks too complicated, I'd just check notifications callback received, using 'struct message' data, same way WM_NOTIFY tests work. > + > +/* Create a message to test against */ > +static struct message create_test_message(UINT message, WPARAM wParam, LPARAM lParam) > +{ > + struct message mes; > + > + mes.message = WM_TD_CALLBACK; > + mes.id = message; > + mes.flags = sent|wparam|lparam|id; > + mes.wParam = wParam; > + mes.lParam = lParam; > + > + return mes; > +} > + > +/* Our only way to get a button handle, since GetDlgItem and FindWindowEx don't work for the official taskdialog */ > + > +static HWND taskdialog_child; > +BOOL CALLBACK enum_taskdialog_children_proc(HWND hwnd, LPARAM lParam) > +{ > + CHAR text[100]; > + const CHAR *title = (const CHAR *)lParam; > + > + GetWindowTextA(hwnd, text, sizeof(text)); > + > + if(lstrcmpA(text, title) == 0) > + { > + taskdialog_child = hwnd; > + return FALSE; > + } > + > + return TRUE; > +} > + > +static HWND get_child_from_title(HWND hwnd_parent, const CHAR *title) > +{ > + taskdialog_child = NULL; > + EnumChildWindows(hwnd_parent, enum_taskdialog_children_proc, (LPARAM)title); > + return taskdialog_child; > +} I don't think we need to get that deep. > > + init_msg_sequences(sequences, NUM_MSG_SEQUENCES); > + test_TaskDialogIndirect(); This name is too generic, in a test function like that you can test invalid parameter handling for example. Then have another function for callback notifications, and another one for TDM_* messages. I think for first iteration test should simply check creation/destruction notifications and callback user data value.

2 1

Re: [PATCH] ws2_32: Also check the FQDN when injecting the local IP.
by Alexandre Julliard 28 Aug '17

28 Aug '17

Stefan Dösinger <stefan(a)codeweavers.com> writes: > @@ -6576,6 +6592,8 @@ int WINAPI WS_getaddrinfo(LPCSTR nodename, LPCSTR servname, const struct WS_addr > > hostname = get_hostname(); > if (!hostname) return WSA_NOT_ENOUGH_MEMORY; > + fqdn = get_fqdn(); > + if (!fqdn) return WSA_NOT_ENOUGH_MEMORY; It seems to me that you shouldn't need to retrieve both separately, the FQDN contains the hostname already. -- Alexandre Julliard julliard(a)winehq.org

1 0

Issue with suspended process
by Jon Doron 25 Aug '17

25 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

3 3

Re: [PATCH resend] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Nikolay Sivov 25 Aug '17

25 Aug '17

On 03.08.2017 18:17, Alex Henrie wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42468 > > To be clear, I am not trying to fix all whitespace processing issues. I > am only trying to prevent "" from being erroneously inserted into > the output. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > dlls/msxml3/tests/domdoc.c | 106 +++++++++++++++++++++++++++++++++++++-------- > dlls/msxml3/text.c | 42 +++++++++++++++--- > 2 files changed, 122 insertions(+), 26 deletions(-) > > diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c I think it needs more tests, for xml:space and put_preserveWhiteSpace(). > diff --git a/dlls/msxml3/text.c b/dlls/msxml3/text.c > index 3a12d4bf62..6814c1f817 100644 > --- a/dlls/msxml3/text.c > +++ b/dlls/msxml3/text.c > @@ -519,10 +519,15 @@ static HRESULT WINAPI domtext_get_xml( > BSTR* p) > { > domtext *This = impl_from_IXMLDOMText( iface ); > + HRESULT hr; > > TRACE("(%p)->(%p)\n", This, p); > > - return node_get_xml(&This->node, FALSE, p); > + hr = node_get_xml(&This->node, FALSE, p); > + if (hr == S_OK) > + *p = EnsureCorrectEOL(*p); > + > + return hr; Will node_get_xml(..., TRUE, ...) work? > } > > static HRESULT WINAPI domtext_transformNode( > @@ -616,15 +621,38 @@ static HRESULT WINAPI domtext_put_data( > BSTR data) > { > domtext *This = impl_from_IXMLDOMText( iface ); > - static const WCHAR rnW[] = {'\r','\n',0}; > + BSTR normalized_data = NULL; > + HRESULT hr; > + int i, j; > > TRACE("(%p)->(%s)\n", This, debugstr_w(data)); > > - if (data && !strcmpW(rnW, data)) > - This->node.node->name = xmlStringTextNoenc; > - else > - domtext_reset_noenc(This); > - return node_set_content(&This->node, data); > + if (data) > + { > + /* normalize line endings */ > + normalized_data = SysAllocStringLen(NULL, SysStringLen(data)); > + if (!normalized_data) return E_OUTOFMEMORY; > + for (i = 0, j = 0; data[i]; i++) > + { > + if (data[i] == '\r') > + { > + if (data[i + 1] == '\n') > + continue; /* change \r\n to just \n */ > + else > + normalized_data[j] = '\n'; /* change \r by itself to \n */ > + } > + else > + normalized_data[j] = data[i]; > + j++; > + } > + normalized_data[j] = 0; The only purpose for that is to inhibit libxml2 escaping, right? Could this be done the way it is now, but with something more elaborate than comparing to "\r\n"? > + } > + > + domtext_reset_noenc(This); > + hr = node_set_content(&This->node, normalized_data); > + > + SysFreeString(normalized_data); > + return hr; > } > > static HRESULT WINAPI domtext_get_length( >

2 1

Re: [v2] d3d9/tests: Add test for SW shader in MVP mode.
by Marvin 25 Aug '17

25 Aug '17

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=32837 Your paranoid android. === w8 (32 bit visual) === visual.c:7952: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080

2 1

Issue with suspended process
by Jonathan Doron 25 Aug '17

25 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

2 1

Re: [PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.
by Henri Verbeet 24 Aug '17

24 Aug '17

On 23 August 2017 at 16:17, Paul Gofman <gofmanp(a)gmail.com> wrote: > -static IDirect3DDevice9 *create_device(IDirect3D9 *d3d, HWND device_window, HWND focus_window, BOOL windowed) > +static IDirect3DDevice9 *create_device_behavior(IDirect3D9 *d3d, HWND device_window, HWND focus_window, > + BOOL windowed, DWORD behavior_flags) It's perhaps a bit of an uncomfortable change to make due to the number of tests in visual.c, but ideally this would match the version of create_device() in device.c. Lacking that, there are a few tests in visual.c that just call IDirect3D9_CreateDevice() directly. In either case, we'd like device.c and visual.c to converge instead of diverge. > + hr = IDirect3DDevice9_SetVertexShader(device, NULL); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); > + hr = IDirect3DDevice9_SetVertexShader(device, pure_sw_shader); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); Is resetting the shader to NULL really needed here? > + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); > + todo_wine > + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); > + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad)); > + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr); Odd. > + /* wined3d does not guarantee 0 for out of bounds constant access by default. */ IIRC neither does Windows, in general. Matteo may have a better recollection of the details, but IIRC while it mostly works out in practice on current NVIDIA hardware, it doesn't necessarily everywhere else, and is essentially hardware/driver specific behaviour.

2 2

Re: [PATCH 2/2] mshtml: Properly expose IHTMLDocument6 interface to scripts.
by Marvin 24 Aug '17

24 Aug '17

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=32807 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Issue with suspended process
by Jonathan Doron 24 Aug '17

24 Aug '17

Hi all, I have an application that is creating a process suspended and then using detours modifies the imports of the new process prior to resuming it (the application is called Tracker.exe and is part of MS toolchain, it can be retrieved in MS EWDK package) The application does not work as it should on Wine, as the DLL its trying to inject to the new process is not getting loaded, I tracked down the problem here are the technical details: When a new process is created, after all the other set-ups its running the function: dlls/ntdll/server.c!server_init_process_done This function sets up the handlers for the different signals (signal_init_process) and makes a wine_server_call init_process_done. In the wine server function server/process.c!init_process_done a check is made to see if the thread of the process needs to be suspended (stop_thread_if_suspended) and indeed when a process is created with the CREATE_SUSPENDED flag, thread->suspend == 1, process->suspend == 0, and the process state is done at this point). This makes the wine_server call to stop_thread which will send a SIGUSR1 signal to the new process. The new process however still have the signals in a block state, thus not processing the signal and putting it into a pending state. The next thing that happens is LdrInitializeThunk continues and fixing up the EXE module imports, this is not the same behavior as Windows in which the Ldr only starts running after the thread has resumed. Only later signals are being unblocked and then the thread enters a suspended state, the function which unblocks the signals today is attach_process_dlls The interesting thing is that in the past Wine behavior was correct and the change that basically broke it is commit: commit 0f5fc117a2320bb7e21b9ae4d07d717bed413171 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Nov 19 14:27:07 2007 +0100 ntdll: Unblock signals in process init only after the dlls have been imported. Unfortunately though there I could not find any reason why the change was made or if there is any bug that it fixed back then (2007) . I was thinking that perhaps in order to resolve the problem and be closer in the behavior to Windows would be to unblock the signals just before server_init_process_done returns. Please let me know if you have any other questions, will be happy to resolve this now that we have all the information laid out. Thanks, -- Jon.

1 0

Filling in ARM64 ABI structs
by Martin Storsjö 23 Aug '17

23 Aug '17

Hi, I've noticed that some of the ABI specific structs for ARM64 are incomplete or missing - namely _JUMP_BUFFER in include/msvcrt/setjmp.h (not yet available) and _CONTEXT (not missing but only has got made-up content so far) and associated CONTEXT_* defines, and structs like RUNTIME_FUNCTION and UNWIND_HISTORY_TABLE in include/winnt.h. As far as I know, these structs should be available in the win10 sdk. (MSDN doesn't have them but just says to refer to winnt.h for the CONTEXT struct at least). In order not to accidentally make myself uneligible for contributing code on these matters, I've tried to not look at the sdk. Can someone help out with completeing these data structures according to whatever clean-room guidelines are required? When looking at existing code with git blame, they mostly appear as such without any further explanation on how they were derived, so I'm taking the more cautious route here. // Martin

2 2

Re: [PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.
by Marvin 23 Aug '17

23 Aug '17

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=32778 Your paranoid android. === w8 (32 bit visual) === visual.c:7975: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 === w1064 (64 bit visual) === visual.c:8023: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

2 1

JACK driver for WINE
by Jeremy Banks 23 Aug '17

23 Aug '17

Hello all, I am wanting to output the audio from various applications run in WINE to the JACK server on my computer. I would prefer that WINE do this natively instead of routing across various wrapper setups with ALSA or PulseAudio. I have looked on the WINE wiki page for sound[1] and see that there was a JACK driver at one point, but it was removed when the driver architecture was changed. I am interested in trying to implement a driver to get WINE talking to JACK natively. The wiki says to ask on the mailing list before starting such a project, and so here I am. I have downloaded a local copy of the source repo and have started browsing through its contents. Being unfamiliar with the WINE codebase, which portions of the source should I start studying first? I am interested in studying the drivers for ALSA, OSS, and Pulse for comparison; do those live in a particular place in the source tree? I'm presently looking on the Wiki for other helpful information, but is there some particular documentation I should be studying for this endeavor? Regards, Jeremy Banks 1. https://wiki.winehq.org/Sound#Backend_information

3 4

Killing https://www.winehq.org/docs/wineusr-guide
by Austin English 22 Aug '17

22 Aug '17

Hi Rosanne, I saw on https://bugs.winehq.org/show_bug.cgi?id=43525 that wineusr-guide is deprecated (which I realized), but I didn't realize it was still up. Would you (or anyone else) be opposed to completely removing it? IMO it's doing more harm than good, especially without a deprecated notice on it. -- -Austin GPG: 14FB D7EA A041 937B

3 3

Re: [PATCH 4/4] mshtml: Added delayed DispatchEx init support and use it to expose IHTMLDOMNode3 from document node.
by Marvin 21 Aug '17

21 Aug '17

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=32755 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer
by Alistair Leslie-Hughes 18 Aug '17

18 Aug '17

Hi Fabian, On 18/08/17 11:25, Fabian Maurer wrote: > + if (IsEqualGUID(riid, &IID_IUnknown) > + || IsEqualGUID(riid, &IID_IBaseFilter)) > + { Your missing some IID checks here. Alistair.

2 3

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer
by Nikolay Sivov 18 Aug '17

18 Aug '17

On 18.08.2017 4:25, Fabian Maurer wrote: > +static const IBaseFilterVtbl IBaseFilter_Vtbl = > +{ > + /*** IUnknown methods ***/ > + EnhancedVideoRendererImpl_QueryInterface, > + EnhancedVideoRendererImpl_AddRef, > + EnhancedVideoRendererImpl_Release, > + /*** IPersist methods ***/ > + EnhancedVideoRendererImpl_GetClassID, > + /*** IMediaFilter methods ***/ > + EnhancedVideoRendererImpl_Stop, > + EnhancedVideoRendererImpl_Pause, > + EnhancedVideoRendererImpl_Run, > + EnhancedVideoRendererImpl_GetState, > + EnhancedVideoRendererImpl_SetSyncSource, > + EnhancedVideoRendererImpl_GetSyncSource, > + /*** IBaseFilter methods ***/ > + EnhancedVideoRendererImpl_EnumPins, > + EnhancedVideoRendererImpl_FindPin, > + EnhancedVideoRendererImpl_QueryFilterInfo, > + EnhancedVideoRendererImpl_JoinFilterGraph, > + EnhancedVideoRendererImpl_QueryVendorInfo > +}; Shouldn't it use strmbase? > +HRESULT EnhancedVideoRendererImpl_Create(IUnknown *pUnkOuter, void **ppObj) > +{ > + EnhancedVideoRendererImpl *object; > + > + TRACE("(%p,%p)\n", pUnkOuter, ppObj); > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(EnhancedVideoRendererImpl)); > + if (!object) > + return E_OUTOFMEMORY; > + > + object->IBaseFilter_iface.lpVtbl = &IBaseFilter_Vtbl; > + object->ref = 1; > + > + *ppObj = &object->IBaseFilter_iface; > + > + return S_OK; > +} This needs at test for aggregation support. > +typedef struct { > + IBaseFilter IBaseFilter_iface; > + LONG ref; > +} EnhancedVideoRendererImpl; Does this need to be exposed? > + TRACE("(0x%p, %d, %p)\n", instance, reason, reserved); > + Format looks wrong. > - case DLL_WINE_PREATTACH: > - return FALSE; /* prefer native version */ It still doesn't do anything. > +static HRESULT WINAPI XFCF_QueryInterface(LPCLASSFACTORY iface,REFIID riid, void **ppobj) > +{ What does XF prefix stand for? Also please use consistent type names and formatting. > +/******************************************************************************* > + * Retrieves class object from a DLL object > + * > + * NOTES > + * Docs say returns STDAPI > + * > + * PARAMS > + * rclsid [I] CLSID for the class object > + * riid [I] Reference to identifier of interface for class object > + * ppv [O] Address of variable to receive interface pointer for riid > + * > + * RETURNS > + * Success: S_OK > + * Failure: CLASS_E_CLASSNOTAVAILABLE, E_OUTOFMEMORY, E_INVALIDARG, > + * E_UNEXPECTED > + */ > +HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, void **ppv) I personally don't think this export needs a documentation header. > + if ( !IsEqualGUID( &IID_IClassFactory, riid ) > + && ! IsEqualGUID( &IID_IUnknown, riid) ) > + return E_NOINTERFACE; I think it's better to let QueryInterface handle this. > + for (i=0; i < sizeof(object_creation)/sizeof(object_creation[0]); i++) > + { > + if (IsEqualGUID(object_creation[i].clsid, rclsid)) > + break; > + } > + > + if (i == sizeof(object_creation)/sizeof(object_creation[0])) > + { > + FIXME("%s: no class found.\n", debugstr_guid(rclsid)); > + return CLASS_E_CLASSNOTAVAILABLE; > + } Does it support any other CLSIDs? This seems like too much. Static factory instance will work too I suppose.

2 3

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Sebastian Lackner 18 Aug '17

18 Aug '17

On 18.08.2017 13:41, Dmitry Timoshkov wrote: > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > dlls/user32/tests/dialog.c | 6 ++++++ > dlls/user32/tests/resource.rc | 6 ++++++ > 2 files changed, 12 insertions(+) > There is no need to resubmit, the patches are exactly identical to what Andrew submitted. Also, why the hell is there my sign-off? It is apparently not fine for you when Andrew wants to take responsiblity for your patches, but you expect me to take responsibility for it, without confirming that I'm fine with it first? For other people I would now start to explain that Wine Staging also accepts workarounds and imperfect patches, and that the addition to Wine Staging does not imply my sign-off, but I'm pretty sure you are well aware of it...

2 1

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Marvin 18 Aug '17

18 Aug '17

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=32738 Your paranoid android. === w7u (32 bit msg) === msg.c:5084: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088 msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 11: the msg 0x001c was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 12: the msg 0x0086 was expected, but got msg 0x001c instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 13: the msg 0x0006 was expected, but got msg 0x0086 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 18: the msg 0x0009 was expected, but got msg 0x0006 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 19: the msg 0x0007 was expected, but got msg 0x0009 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 20: the msg 0x0047 was expected, but got msg 0x0007 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 21: the msg 0x0003 was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 22: the msg sequence is not complete: expected 0000 - actual 0003 === wvistau64 (32 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === w864 (32 bit broadcast) === The task timed out === w8 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit sysparams) === sysparams.c:206: Test failed: Unexpected WM_DISPLAYCHANGE message === w864 (64 bit sysparams) === The task timed out

1 0

Re: [v5 PATCH 1/3] dwrite: Protect cached fontface list when accessed from multiple threads
by Anton Romanov 18 Aug '17

18 Aug '17

On Tue, Aug 15, 2017 at 4:40 AM, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 15 ++++++++--- > dlls/dwrite/font.c | 30 +++++++++++++-------- > dlls/dwrite/main.c | 63 +++++++++++++++++++++++++++++++------------- > 3 files changed, 74 insertions(+), 34 deletions(-) > Since apps seem to freely use fontface interfaces from multiple threads - Isn't the following race possible? +----------------+------------------------+-------------------------+ | T1 | T2 | T3 | +----------------+------------------------+-------------------------+ | Release | | | | Decrement | | | | == 0 -> True | | | | | Query_Interface/AddRef | | | | Increment | | | | | Release | | | | Decrement | | free(cached) | | | | factory_unlock | | | | | | use after free (cached) | +----------------+------------------------+-------------------------+

3 7

Re: [PATCH] comctl32/listview: Keep the active item selected after hwndEdit is destroyed
by Nikolay Sivov 17 Aug '17

17 Aug '17

On 10.08.2017 7:54, Hugh McMaster wrote: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/comctl32/listview.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c > index 73eceff509..05bd375571 100644 > --- a/dlls/comctl32/listview.c > +++ b/dlls/comctl32/listview.c > @@ -10216,6 +10216,7 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, IN > static BOOL bGroupSelect = TRUE; > POINT pt = { x, y }; > INT nItem; > + BOOL editing = FALSE; > > TRACE("(key=%hu, X=%u, Y=%u)\n", wKey, x, y); > > @@ -10309,11 +10310,20 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, IN > else > { > if (!infoPtr->bFocus) > + { > + editing = IsWindow(infoPtr->hwndEdit); > SetFocus(infoPtr->hwndSelf); > + } > > - /* remove all selections */ > + /* keep the active item selected after hwndEdit is destroyed, or deselect all items */ > if (!(wKey & MK_CONTROL) && !(wKey & MK_SHIFT)) > - LISTVIEW_DeselectAll(infoPtr); > + { > + if (editing) > + LISTVIEW_DeselectAllSkipItem(infoPtr, infoPtr->nFocusedItem); > + else > + LISTVIEW_DeselectAll(infoPtr); > + } > + Maybe this could be reduced to a single unconditional call: LISTVIEW_DeselectAllSkipItem(infoPtr, infoPtr->nEditLabelItem); > ReleaseCapture(); > } > >

1 0

Re: [PATCH 1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Marvin 17 Aug '17

17 Aug '17

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=32717 Your paranoid android. === wvistau64 (32 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === w864 (64 bit broadcast) === The task timed out === w8 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w864 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1

2 1

Re: [PATCH 1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.
by Dmitry Timoshkov 17 Aug '17

17 Aug '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > From: Dmitry Timoshkov <dmitry(a)baikal.ru> > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > dlls/user32/tests/dialog.c | 6 ++++++ > dlls/user32/tests/resource.rc | 6 ++++++ > 2 files changed, 12 insertions(+) I think that I've expressed my opinion clear enough in my response to your private e-mail that I don't allow you to send my patches included in the wine-staging tree to winehq, and explained well enough my reasoning. The only person who has my permission is Sebastian Lackner - wine-staging maintainer. I'm too sometimes sending the patches, but I have to admit that's pretty rare cases these days, and usually that's not my personal decision. Please try to find your own areas to work on, and stop stealing patches created by somebody else, especially when directly asked not to. -- Dmitry.

3 12

Re: [v5 PATCH 1/3] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 17 Aug '17

17 Aug '17

On 15.08.2017 13:40, Nikolay Sivov wrote: > if (cached_key_size == key_size && !memcmp(cached_key, key, key_size)) { > + if (FAILED(hr = IDWriteFontFace4_QueryInterface(cached->fontface, riid, obj))) > + WARN("Failed to get %s from fontface, hr %#x.\n", debugstr_guid(riid), hr); > + > TRACE("returning cached fontface %p\n", cached->fontface); > - *font_face = (IDWriteFontFace*)cached->fontface; > - return S_OK; > + break; > } > } There is still a minor problem (no race condition though). Lets assume a fontface is just about to be destroyed, then the code above will abort enumeration after reaching the fontface with refcount == 0. Lets assume the newly created fontface is added after the old one, then all following attempts to get the fontface from the cache will fail, until the old one has finally be removed. This issue can be avoided by doing "continue;" instead of "break;" for E_FAIL.

1 0

Re: [PATCH] kernel32/nls: Added LOCALE_SNAN entries
by Henri Verbeet 17 Aug '17

17 Aug '17

On 17 August 2017 at 00:28, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > diff --git a/dlls/kernel32/nls/far.nls b/dlls/kernel32/nls/far.nls > index 0f5b17068b..2baf289223 100644 > --- a/dlls/kernel32/nls/far.nls > +++ b/dlls/kernel32/nls/far.nls > @@ -124,6 +124,7 @@ STRINGTABLE LANGUAGE LANG_PERSIAN, SUBLANG_DEFAULT > LOCALE_SMONTHNAME13 "" > LOCALE_SMONTHOUSANDSEP "٬" > LOCALE_SNAME "fa-IR" > + LOCALE_SNAN "ﻥﺎﻋﺩﺩ" That doesn't look right. According to http://www.unicode.org/cldr/charts/latest/by_type/numbers.symbols.html#2a6d… "ناعدد" is correct, but the above is using the Presentation Forms-B characters.

2 1

Re: [PATCH] msi: Print error number when GetProcAddress fails
by Hans Leidekker 17 Aug '17

17 Aug '17

On Wed, 2017-08-16 at 17:20 +0100, Carlos Palminha wrote: > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c > index 922927e8903..cfdfe852efb 100644 > --- a/dlls/msi/custom.c > +++ b/dlls/msi/custom.c > @@ -608,7 +608,7 @@ static DWORD ACTION_CallDllFunction( const GUID *guid ) > ERR("failed to create handle for %p\n", remote_package ); > } > else > - ERR("GetProcAddress(%s) failed\n", debugstr_w( function ) ); > + ERR("GetProcAddress(%s) failed: %d\n", debugstr_w( function ), GetLastError() ); > > FreeLibrary(hModule); This doesn't appear to add any useful information. Our implementation of GetProcAddress sets either ERROR_PROC_NOT_FOUND or ERROR_MOD_NOT_FOUND and LoadLibrary must have succeeded before reaching this code.

3 2

Re: [PATCH v3] user32/combo: Set listview popup height correctly and add tests
by Nikolay Sivov 16 Aug '17

16 Aug '17

It's a listbox, not listview.

2 1

D3D8 Software Vertex Processing
by Jan Havran 16 Aug '17

16 Aug '17

Hello, i am trying to fix #9337 [1] for VietCong game (game is crashing during loading 3D objects) for which exists community util which fixes it. This util was made to fix issues for VC on newer Windows - fixing this bug in Wine was just side effect. Anyway, i have talked to creator of this util (Brchi from The Archi Family) about it. The game originally use software vertexes only, but he edited the game and switched to hardware vertexes (which increased FPS rapidly on Windows), but he was facing the same problem - the game was crashing. Fortunately, he found that the game uses different code branches when hardware vertex are used - developers probably never finished support for hardware vertexes and that is reason why it is crashing when HW vertexes are used. He fixed game code so it works with HW vertexes (even in Wine). The problem is that game uses only SW vertexes and these unfinished branches are called only when HW vertexes are used, which is not a problem for Windows (which support SW vertexes). But for some reason Wine uses HW vertexes even if game wants to use SW vertexes. Thats why the game works in Wine only with this util applied. I can see from report [2] that game tries to use SW vertexes, which are unsupported by Wine. Can I simply edit some code in Wine (some D3D flag) so the game will think that it use SW vertexes, so after checking some flags by game it will not jump into some unfinished branches for HW vertexes? I just want to test if it is really caused by this. From application view, what is difference between using SW and HW vertexes, do you think that using HW vertexes with code done for SW vertexes will work? [1] https://bugs.winehq.org/show_bug.cgi?id=9337 [2] https://bugs.winehq.org/show_bug.cgi?id=9337#c35 Thanks, Jan

2 6

Re: [PATCH] gdiplus: Implement transform matrix for line gradient brushes
by Vincent Povirk 16 Aug '17

16 Aug '17

I wrote a test program (attached), and it doesn't work when the start point isn't 0,0. > + if (line->startpoint.X == line->endpoint.X) > + t = line->startpoint.Y > line->endpoint.Y ? 3.f * M_PI / 2.f : M_PI / 2.f; > + else > + t = atanf((line->endpoint.Y - line->startpoint.Y) / (line->endpoint.X - line->startpoint.X)); > + > + t_cos = cosf(t); > + t_sin = sinf(t); This seems like an oddly complicated and fragile way to normalize a vector.

2 1

Re: [PATCH] avrt: Add AvQuerySystemResponsiveness stub.
by Huw Davies 16 Aug '17

16 Aug '17

On Wed, Aug 16, 2017 at 02:22:13PM +0300, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > dlls/avrt/avrt.spec | 2 +- > dlls/avrt/main.c | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/dlls/avrt/avrt.spec b/dlls/avrt/avrt.spec > index 640a12ac22..a7dfc42879 100644 > --- a/dlls/avrt/avrt.spec > +++ b/dlls/avrt/avrt.spec > @@ -1,4 +1,4 @@ > -@ stub AvQuerySystemResponsiveness > +@ stdcall AvQuerySystemResponsiveness(long ptr) > @ stdcall AvRevertMmThreadCharacteristics(long) > @ stub AvRtCreateThreadOrderingGroup > @ stub AvRtCreateThreadOrderingGroupExA > diff --git a/dlls/avrt/main.c b/dlls/avrt/main.c > index 8a110250ae..68e44d6098 100644 > --- a/dlls/avrt/main.c > +++ b/dlls/avrt/main.c > @@ -83,6 +83,12 @@ HANDLE WINAPI AvSetMmThreadCharacteristicsW(LPCWSTR TaskName, LPDWORD TaskIndex) > return (HANDLE)0x12345678; > } > > +BOOL WINAPI AvQuerySystemResponsiveness(HANDLE AvrtHandle, PULONG SystemResponsivenessValue) > +{ > + FIXME("(%p, %p): stub\n", AvrtHandle, SystemResponsivenessValue); > + return TRUE; > +} > + If you're going to return TRUE, you need to initialize *SystemResponsivenessValue to something meaningful. While you're at it, we generally prefer 'ULONG *' to 'PULONG', although I realise there are several of the latter form in this file. Huw.

2 3

Re: [PATCH v2] server: Use queue_irp() also for synchronous IRPs.
by Alexandre Julliard 16 Aug '17

16 Aug '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > server/device.c | 46 ++++++++++++++++++++-------------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) It's probably a matter of taste, but this doesn't look to me like an improvement. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v4 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 16 Aug '17

16 Aug '17

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=32694 Your paranoid android. === wvistau64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === wvistau64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8

1 0

Re: [PATCH] usp10: fall back to 'dflt' language if shaping language tag not found
by Henri Verbeet 15 Aug '17

15 Aug '17

On 14 August 2017 at 20:04, Aric Stewart <aric(a)codeweavers.com> wrote: > + script = get_opentype_script(hdc,psa,psc,FALSE); Does that do anything? If I'm reading the code correctly that will set it to the same value it already was. (In which case most of this would simplify to "if (!script) script = MS_MAKE_TAG('l','a','t','n');")

2 2

Re: [PATCH v3 5/5] ieframe: Add stub implementation of IInternetExplorerManager.
by Jacek Caban 15 Aug '17

15 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > v2: register IE and IEM separately, and the latter with SINGLEUSE > (thanks Jacek) > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/ieframe/ieframe.h | 5 +- > dlls/ieframe/ieframe_main.c | 35 ++++--------- > dlls/ieframe/iexplore.c | 111 ++++++++++++++++++++++++++++++++++++++--- > dlls/ieframe/tests/ie.c | 21 ++++++++ > programs/iexplore/iexplore.inf | 3 ++ > 5 files changed, 142 insertions(+), 33 deletions(-) > > diff --git a/dlls/ieframe/ieframe.h b/dlls/ieframe/ieframe.h > index d1f1c5daf24..c690d5c6878 100644 > --- a/dlls/ieframe/ieframe.h > +++ b/dlls/ieframe/ieframe.h > @@ -312,13 +312,16 @@ TID_LIST > } tid_t; > > HRESULT get_typeinfo(tid_t,ITypeInfo**) DECLSPEC_HIDDEN; > -HRESULT register_class_object(BOOL) DECLSPEC_HIDDEN; > > HRESULT WINAPI CUrlHistory_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI InternetExplorer_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI InternetShortcut_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI WebBrowser_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > HRESULT WINAPI WebBrowserV1_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN; > + > +extern IClassFactory InternetExplorerFactory; > +extern IClassFactory InternetExplorerManagerFactory; This is missing DECLSPEC_HIDDEN. > > +/****************************************************************** > + * IInternetExplorerManager implementation > + */ > +struct internet_explorer_manager { > + IInternetExplorerManager IInternetExplorerManager_iface; > + LONG ref; > +}; Please use the type from public declaration (coclass). See how InternetExplorer object is declared. > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory *iface, IUnknown *pOuter, REFIID riid, void **ppv) > +{ > + struct internet_explorer_manager *ret; > + > + TRACE("(%p %s %p)\n", pOuter, debugstr_guid(riid), ppv); > + > + if (!(ret = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*ret)))) > + return E_OUTOFMEMORY; > + > + ret->IInternetExplorerManager_iface.lpVtbl = &InternetExplorerManager_vtbl; > + return IInternetExplorerManager_QueryInterface(&ret->IInternetExplorerManager_iface, riid, ppv); > +} This leaks the object if QueryInterface fails. The usual way to handle is to set initial ref to 1 and release the object after QI() call. > +static void test_InternetExplorerManager(void) > +{ > + IUnknown *unk; > + ULONG ref; > + HRESULT hres; > + > + hres = CoCreateInstance(&CLSID_InternetExplorerManager, NULL, CLSCTX_LOCAL_SERVER, > + &IID_IInternetExplorerManager, (void**)&unk); > + ok(hres == S_OK || hres == REGDB_E_CLASSNOTREG, "Could not create InternetExplorerManager instance: %08x\n", hres); Use broken() for failure with older versions. > + > + if(hres != S_OK) > + return; This could use a win_skip(). Thanks, Jacek

1 0

Re: [PATCH v3 4/5] ieframe: Parse multiple command-line arguments.
by Jacek Caban 15 Aug '17

15 Aug '17

Hi Zebediah, On 10.08.2017 20:52, Zebediah Figura wrote: > + if (!strncmpiW(cmdline, embeddingW, length)) > + embedding = TRUE; > + else if (!strncmpiW(cmdline, nohomeW, length)) > + nohome = TRUE; > + else if (*cmdline == '-') > + FIXME("ignoring unknown option %s\n", debugstr_wn(cmdline, length)); It's probably not very important, but with native IE unknown options seem to be treated as URLs (iexplore.exe -test tries to open "-test" URL). Thanks, Jacek

1 0

Re: [PATCH 2/3] include: Add CryptProtectMemory and CryptUnprotectMemory.
by Michael Stefaniuc 15 Aug '17

15 Aug '17

On 08/15/2017 05:00 PM, Alex Henrie wrote: > Needed for PuTTY to compile with Winelib. > > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> > --- > include/wincrypt.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/wincrypt.h b/include/wincrypt.h > index abbe99f245..058798c4f4 100644 > --- a/include/wincrypt.h > +++ b/include/wincrypt.h > @@ -4428,6 +4428,9 @@ BOOL WINAPI CryptUnprotectData( DATA_BLOB* pDataIn, LPWSTR* ppszDataDescr, > DATA_BLOB* pOptionalEntropy, PVOID pvReserved, > CRYPTPROTECT_PROMPTSTRUCT* pPromptStruct, DWORD dwFlags, DATA_BLOB* pDataOut ); > > +BOOL WINAPI CryptProtectMemory(void *pData, DWORD cbData, DWORD dwFlags); > +BOOL WINAPI CryptUnrotectMemory(void *pData, DWORD cbData, DWORD dwFlags); Isn't that missing a p^ ? bye michael > + > DWORD WINAPI CertGetNameStringA(PCCERT_CONTEXT pCertContext, DWORD dwType, > DWORD dwFlags, void *pvTypePara, LPSTR pszNameString, DWORD cchNameString); > DWORD WINAPI CertGetNameStringW(PCCERT_CONTEXT pCertContext, DWORD dwType, >

2 1

Re: [3/8] bcrypt: Add a stub implementation of BCryptEncrypt.
by Jacek Caban 15 Aug '17

15 Aug '17

Hi Hans, On 15.08.2017 14:51, Hans Leidekker wrote: > diff --git a/dlls/bcrypt/bcrypt_main.c b/dlls/bcrypt/bcrypt_main.c > index 3d70c15e7c..ccab676c1a 100644 > --- a/dlls/bcrypt/bcrypt_main.c > +++ b/dlls/bcrypt/bcrypt_main.c > @@ -515,7 +515,7 @@ NTSTATUS WINAPI BCryptGetProperty( BCRYPT_HANDLE handle, LPCWSTR prop, UCHAR *bu > NTSTATUS WINAPI BCryptSetProperty( BCRYPT_HANDLE handle, const WCHAR *prop, UCHAR *value, ULONG size, ULONG flags ) > { > FIXME( "%p, %s, %p, %u, %08x\n", handle, debugstr_w(prop), value, size, flags ); > - return STATUS_SUCCESS; > + return STATUS_NOT_IMPLEMENTED; I think this part was meant to be in patch 2/8. Thanks, Jacek

1 0

Re: [PATCH] user32/combo: Set listview popup height correctly and add tests
by Marvin 14 Aug '17

14 Aug '17

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=32672 Your paranoid android. === wxppro (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w2003std (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === wvistau64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w2008s64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w7u (32 bit combo) === combo.c:786: Test failed: Test 2, expected list height to be 16, got 0 combo.c:786: Test failed: Test 3, expected list height to be 16, got 18 combo.c:786: Test failed: Test 4, expected list height to be 32, got 18 combo.c:786: Test failed: Test 5, expected list height to be 32, got 36 combo.c:786: Test failed: Test 6, expected list height to be 32, got 36 combo.c:786: Test failed: Test 7, expected list height to be 32, got 36 combo.c:786: Test failed: Test 8, expected list height to be 32, got 36 combo.c:786: Test failed: Test 11, expected list height to be 16, got 0 combo.c:786: Test failed: Test 12, expected list height to be 16, got 18 combo.c:786: Test failed: Test 13, expected list height to be 32, got 18 combo.c:786: Test failed: Test 14, expected list height to be 48, got 36 combo.c:781: Test failed: Test 15, expected list height to be 48, got 54 combo.c:786: Test failed: Test 16, expected list height to be 64, got 54 combo.c:786: Test failed: Test 17, expected list height to be 64, got 72 combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w7pro64 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w8 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w864 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === w1064 (32 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 00000000 === wvistau64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w2008s64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w7pro64 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w864 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000 === w1064 (64 bit combo) === combo.c:315: Test failed: Focus not on Main Window, instead on 0000000000000000

2 1

Re: [1/3] shell32: Implement SHCreateItemFromRelativeName.
by Huw Davies 14 Aug '17

14 Aug '17

On Sat, Aug 12, 2017 at 11:42:40PM +0800, Jactry Zeng wrote: > +HRESULT WINAPI SHCreateItemFromRelativeName(IShellItem *parent, PCWSTR name, IBindCtx *pbc, > + REFIID riid, void **ppv) > +{ > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + HRESULT hr; > + > + TRACE("(%p, %p, %p, %s, %p)\n", parent, name, pbc, debugstr_guid(riid), ppv); Use debugstr_w() to trace 'name'. > + > + if(!ppv) > + return E_INVALIDARG; > + *ppv = NULL; > + if(!name) > + return E_INVALIDARG; > + > + hr = SHGetIDListFromObject((IUnknown*)parent, &pidl_folder); > + if(hr != S_OK) > + return hr; > + > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) > + goto cleanup; > + > + if(!_ILIsDesktop(pidl_folder)) > + { > + hr = IShellFolder_BindToObject(desktop, pidl_folder, pbc, &IID_IShellFolder, > + (void**)&folder); You should pass a NULL IBindCtx here. pbc is associated with binding the name. > + if(hr != S_OK) > + goto cleanup; > + } > + > + hr = IShellFolder_ParseDisplayName(folder ? folder : desktop, NULL, pbc, (LPWSTR)name, > + NULL, &pidl, NULL); > + if(hr != S_OK) > + goto cleanup; > + hr = SHCreateItemFromIDList(pidl, riid, ppv); > + > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..cbab88035a 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > @@ -2519,6 +2523,86 @@ static void test_SHCreateShellItem(void) > else > win_skip("No SHCreateItemFromIDList\n"); > > + /* SHCreateItemFromRelativeName */ > + if(pSHCreateItemFromRelativeName && pSHGetKnownFolderPath) > + { > + IShellItem *shellitem_desktop = NULL; > + WCHAR *desktop_path, *displayname; > + WCHAR testfile_path[MAX_PATH] = {0}; > + HANDLE file; > + LPITEMIDLIST pidl_desktop_testfile = NULL; > + int order; > + > + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem_desktop); > + ok(ret == S_OK, "SHCreateShellItem failed: 0x%08x.\n", ret); > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, NULL, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == E_INVALIDARG, "Expected 0x%08x but SHCreateItemFromRelativeName return: 0x%08x.\n", > + E_INVALIDARG, ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + /* Test with a non-existent file */ > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), > + "Expected 0x%08x but SHCreateItemFromRelativeName return: 0x%08x.\n", > + HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + /* Create a file for testing in desktop folder */ > + pSHGetKnownFolderPath(&FOLDERID_Desktop, 0, NULL, &desktop_path); > + lstrcatW(testfile_path, desktop_path); > + myPathAddBackslashW(testfile_path); > + lstrcatW(testfile_path, testfileW); > + file = CreateFileW(testfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFileW failed! Last error: 0x%08x.\n", GetLastError()); > + CloseHandle(file); > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == S_OK, "SHCreateItemFromRelativeName failed: 0x%08x.\n", ret); > + ok(shellitem != NULL, "shellitem was %p.\n", shellitem); > + if(SUCCEEDED(ret)) > + { > + ret = IShellItem_GetDisplayName(shellitem, 0, &displayname); > + ok(ret == S_OK, "IShellItem_GetDisplayName failed: 0x%08x.\n", ret); > + ok(!lstrcmpW(displayname, testfileW), "got wrong display name: %s.\n", wine_dbgstr_w(displayname)); > + > + shellitem2 = (void*)0xdeadbeef; > + ret = pSHCreateItemFromRelativeName(shellitem_desktop, testfileW, NULL, &IID_IShellItem, > + (void**)&shellitem2); > + ok(ret == S_OK, "SHCreateItemFromRelativeName failed: 0x%08x.\n", ret); > + ret = IShellItem_Compare(shellitem, shellitem2, 0, &order); > + ok(ret == S_OK, "IShellItem_Compare failed: 0x%08x.\n", ret); > + ok(!order, "order got wrong value: %d.\n", order); > + IShellItem_Release(shellitem2); > + > + shellitem2 = (void*)0xdeadbeef; > + ret = IShellFolder_ParseDisplayName(desktopfolder, NULL, NULL, testfileW, NULL, > + &pidl_desktop_testfile, NULL); > + ok(ret == S_OK, "ParseDisplayName failed 0x%08x.\n", ret); > + ret = pSHCreateItemFromIDList(pidl_desktop_testfile, &IID_IShellItem, (void**)&shellitem2); > + ret = IShellItem_Compare(shellitem, shellitem2, 0, &order); > + ok(ret == S_OK, "IShellItem_Compare fail: 0x%08x.\n", ret); > + ok(!order, "order got wrong value: %d.\n", order); > + pILFree(pidl_desktop_testfile); > + IShellItem_Release(shellitem2); > + > + IShellItem_Release(shellitem); > + } > + > + DeleteFileW(testfile_path); > + CoTaskMemFree(desktop_path); > + CoTaskMemFree(displayname); dipslayname is uninitialized if the 'if' block isn't executed. Either move this inside the 'if' block, or just get rid of the 'if' condition. We expect the condition to succeed in all cases anyway. Huw.

2 1

Re: [2/3 v4] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 14 Aug '17

14 Aug '17

On Sat, Aug 12, 2017 at 11:42:43PM +0800, Jactry Zeng wrote: > Superseded patch 136380. > > ChangLog: > v4: > - Using SHCreateItemFromRelativeName() for implementing > SHCreateItemInKnownFolder(); > - Fix a logic error in tests. > v3: > - Load SHGetKnownFolderPath() dynamically. > v2: > - Using SHGetKnownFolderIDList() instead of SHGetKnownFolderPath() and > IShellFolder_ParseDisplayName(); > - Add more tests. > > Signed-off-by: Jactry Zeng <jzen g(a)codeweavers.com> > --- > dlls/shell32/shell32.spec | 1 + > dlls/shell32/shellitem.c | 35 +++++++++++++ > dlls/shell32/tests/shlfolder.c | 111 > +++++++++++++++++++++++++++++++++++++++++ > include/shobjidl.idl | 1 + > 4 files changed, 148 insertions(+) > This and patch 3/3 look good to me, so just fix up 1/3 and resend the series. Huw.

1 0

Re: [PATCH] mfplat: Add MFTEnumEx stub.
by Alistair Leslie-Hughes 14 Aug '17

14 Aug '17

Hi Andrey, On 14/08/17 19:59, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > + * MFTEnumEx (mfplat.@) > + */ > +HRESULT WINAPI MFTEnumEx(GUID category, UINT32 flags, const MFT_REGISTER_TYPE_INFO *input_type, > + const MFT_REGISTER_TYPE_INFO *output_type, IMFActivate ***activate, > + UINT32 *pcount) > +{ > + FIXME("(%s, %x, %p, %p, %p, %p): stub\n", debugstr_guid(&category), flags, input_type, > + output_type, activate, pcount); > + return S_OK; > +} If you must return S_OK, please set pcount to 0. (maybe rename pcount to count at the same time). Alistair.

1 0

Re: [PATCH] wined3d: Reduce timeout value to avoid GL_TIMEOUT_EXPIRED on macOS
by Henri Verbeet 12 Aug '17

12 Aug '17

On 12 August 2017 at 11:06, <nneonneo(a)gmail.com> wrote: > - /* Apple seems to be into arbitrary limits, and timeouts larger than > - * 0xfffffffffffffbff immediately return GL_TIMEOUT_EXPIRED. We don't > - * really care and can live with waiting a few μs less. (OS X 10.7.4). */ > + /* Timeouts near 0xffffffffffffffff may immediately return GL_TIMEOUT_EXPIRED, > + * possibly because macOS internally adds some slop to the timer. To avoid this, > + * we use a large number that isn't near the point of overflow (macOS 10.12.5). > + */ > GLenum gl_ret = GL_EXTCALL(glClientWaitSync(fence->object.sync, > - GL_SYNC_FLUSH_COMMANDS_BIT, ~(GLuint64)0xffff)); > + GL_SYNC_FLUSH_COMMANDS_BIT, (GLuint64)((1ULL << 62ULL) - 0xffff))); Adjusting the workaround is probably fine, but ULL literals are not portable. Does something along the lines of "~(GLuint64)0 >> 1" work as well? I don't think we care about the low bits, but note that the patch as it is doesn't actually preserve them.

2 2

Re: [PATCH resend 1/2] krnl386.exe16: Use toupper() instead of RtlUpperChar().
by Alexandre Julliard 12 Aug '17

12 Aug '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/krnl386.exe16/ne_module.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) I don't think that's a good idea. Why do you need that? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH v3 2/5] ieproxy: Add stub DLL.
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > --- a/loader/wine.inf.in > +++ b/loader/wine.inf.in > @@ -2559,6 +2559,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 11,,msxml6.dll > 11,,shdocvw.dll > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll > 16422,Windows Media Player,wmplayer.exe > 16422,Windows NT\Accessories,wordpad.exe > 11,,* > @@ -2613,6 +2614,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 16410,Microsoft, > 16412,Microsoft, > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll Are you sure about it? It's in regular system32 (and syswow64) on my Win10. Jacek

2 2

Re: [PATCH v5 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 16:10, Zebediah Figura wrote: > +cpp_quote("#define INTERNETEXPLORERCONFIGURATION_HOST 0x00000001") This should be a value of INTERNETEXPLORERCONFIGURATION enum. Jacek

1 0

Re: [PATCH v4 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 15:54, Zebediah Figura wrote: > + */ > + > +import "unknwn.idl"; One more thing I didn't notice earlier: this should be ocidl.idl. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Carlos Palminha 11 Aug '17

11 Aug '17

Hi, My patch with test is producing a strange result in Test Bot. I'm getting a ERROR_NOT_ENOUGH_MEMORY when calling LoadLibraryExA only in Vista and Win2008 64bit. Any hint? Regards, C.Palminha On 11-08-2017 11:39, Carlos Palminha wrote: > Test LIBRARY_SEARCH_* flags when loading libraries. > Uses helper from path.c to create the test dll file. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > v3: > - Improved check for Win2003 cases > > v2: > - Split the tests affecting different files > - Tests checks for Win2003 cases > > dlls/kernel32/tests/module.c | 132 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 131 insertions(+), 1 deletion(-) > > diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c > index bb22c24316c..96dd3277c1b 100644 > --- a/dlls/kernel32/tests/module.c > +++ b/dlls/kernel32/tests/module.c > @@ -407,6 +407,136 @@ static void testLoadLibraryEx(void) > FreeLibrary(hmodule); > } > > +/* use some functions and strcutures from loader.c */ > +extern DWORD create_test_dll( const IMAGE_DOS_HEADER *dos_header, UINT dos_size, > + const IMAGE_NT_HEADERS *nt_header, const char *dll_name ); > +extern const IMAGE_NT_HEADERS nt_header_template; > + > +static void testLoadLibraryExSearchFlags(void) > +{ > + HMODULE hmodule; > + char tmpPath[MAX_PATH]; > + char dllName[] = "testFlags.dll"; > + char tmpFullFile[MAX_PATH]; > + char sysFullFile[MAX_PATH]; > + IMAGE_DOS_HEADER dos_header; > + IMAGE_NT_HEADERS nt_header; > + ULONG file_size; > + char fullPath[MAX_PATH]; > + DWORD version; > + BOOL is_win2003 = FALSE; > + > + GetTempPathA(MAX_PATH, tmpPath); > + lstrcpyA(tmpFullFile, tmpPath); > + lstrcatA(tmpFullFile, dllName); > + > + /* check for required function */ > + if (!pSetDllDirectoryA) > + { > + win_skip("SetDllDirectoryA not available\n"); > + return; > + } > + > + /* check if we are in win2003... flags are not supported */ > + version = GetVersion(); > + if ((LOBYTE(LOWORD(version)) == 5) && (HIBYTE(LOWORD(version))==2)) > + is_win2003 = TRUE; > + > + /* try to load test dll from test path before creating it*/ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library %s should not be loaded from %s\n", dllName, tmpPath); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* create test library */ > + lstrcpyA(fullPath, tmpPath); > + lstrcatA(fullPath, dllName); > + > + dos_header.e_magic = IMAGE_DOS_SIGNATURE; > + dos_header.e_lfanew = sizeof(dos_header); > + > + nt_header = nt_header_template; > + > + nt_header.FileHeader.NumberOfSections = 1; > + nt_header.FileHeader.SizeOfOptionalHeader = sizeof(IMAGE_OPTIONAL_HEADER); > + nt_header.OptionalHeader.SectionAlignment = 0x1000; > + nt_header.OptionalHeader.FileAlignment = 0x1000; > + nt_header.OptionalHeader.SizeOfImage = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER) + 0x1000; > + nt_header.OptionalHeader.SizeOfHeaders = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER); > + > + file_size = create_test_dll(&dos_header, sizeof(dos_header), &nt_header, fullPath); > + ok( file_size, "could not create '%s'\n",fullPath); > + > + /* try to load test dll without specifying the test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(NULL); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library should not be loaded from 'NULL' path\n"); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* try to load test dll from test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from %s in Win2003 (flag not supported)\n", tmpPath); > + else > + ok (hmodule != NULL, "Could not load test library %s from %s\n", dllName, tmpPath); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* load non-existing test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + ok(hmodule == NULL, "%s should not be loaded\n", dllName); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* move library from temp path to system32 and delete from old path */ > + GetWindowsDirectoryA(sysFullFile,MAX_PATH); > + lstrcatA(sysFullFile,"\\system32\\"); > + lstrcatA(sysFullFile, dllName); > + MoveFileA(tmpFullFile,sysFullFile); > + DeleteFileA(tmpFullFile); > + > + /* try to load dll using more than one flag */ > + pSetDllDirectoryA(tmpPath); > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS | LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* try to load test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* delete test dll from system32 directory */ > + DeleteFileA(sysFullFile); > + > +} > + > static void testGetDllDirectory(void) > { > CHAR bufferA[MAX_PATH]; > @@ -744,7 +874,6 @@ START_TEST(module) > WCHAR filenameW[MAX_PATH]; > > /* Test if we can use GetModuleFileNameW */ > - > SetLastError(0xdeadbeef); > GetModuleFileNameW(NULL, filenameW, MAX_PATH); > if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) > @@ -766,6 +895,7 @@ START_TEST(module) > testLoadLibraryA_Wrong(); > testGetProcAddress_Wrong(); > testLoadLibraryEx(); > + testLoadLibraryExSearchFlags(); > testGetModuleHandleEx(); > testK32GetModuleInformation(); > } >

1 0

Re: [PATCH v3 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > + > +cpp_quote("DEFINE_GUID(CLSID_InternetExplorerManager, 0xdf4fcc34, 0x067a, 0x4e0a, 0x83,0x52, 0x4a,0x1a,0x50,0x95,0x34,0x6e);") Please use coclass, like in PSDK, instead. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 11 Aug '17

11 Aug '17

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=32633 Your paranoid android. === wvistau64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === wvistau64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8

1 0

Re: [PATCH] comctl32/listview: Keep the active item selected after hwndEdit is destroyed
by Nikolay Sivov 11 Aug '17

11 Aug '17

Could you describe how should I be testing this?

2 1

Re: [PATCH v2 3/5] ieframe: Add stub implementation of IInternetExplorerManager.
by Jacek Caban 10 Aug '17

10 Aug '17

Hi Zebediah, On 08.08.2017 20:04, Zebediah Figura wrote: > HRESULT register_class_object(BOOL do_reg) > { > HRESULT hres; > > - static DWORD cookie; > + static DWORD ie_cookie, iem_cookie; > > if(do_reg) { > hres = CoRegisterClassObject(&CLSID_InternetExplorer, > (IUnknown*)&InternetExplorerFactory, CLSCTX_SERVER, > - REGCLS_MULTIPLEUSE|REGCLS_SUSPENDED, &cookie); > - if (FAILED(hres)) { > + REGCLS_MULTIPLEUSE, &ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRegisterClassObject(&CLSID_InternetExplorerManager, > + (IUnknown*)&InternetExplorerManagerFactory, CLSCTX_SERVER, > + REGCLS_MULTIPLEUSE, &iem_cookie); > + if (FAILED(hres)) > ERR("failed to register object %08x\n", hres); > - return hres; > - } > - > - hres = CoResumeClassObjects(); > - if(SUCCEEDED(hres)) > - return hres; > - > - ERR("failed to resume object %08x\n", hres); > } > - > - return CoRevokeClassObject(cookie); > + else > + { > + hres = CoRevokeClassObject(ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRevokeClassObject(iem_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + } > + return hres; > } Are you sure that's how it's supposed to work? I didn't test it myself, but given how it's registered and documented, I'd expect that CLSID_InternetExplorerManager should live in a different process from CLSID_InternetExplorer. It should be easy to distinguish those processes with -startmanager command line argument. > +static HRESULT WINAPI InternetExplorerManager_CreateObject(IInternetExplorerManager *iface, DWORD config, LPCWSTR url, REFIID riid, void **ppv) > +{ > + FIXME("(%p)->(0x%x, %s, %s, %p) semi-stub\n", iface, config, debugstr_w(url), debugstr_guid(riid), ppv); > + > + return CoCreateInstance(&CLSID_InternetExplorer, NULL, CLSCTX_LOCAL_SERVER, riid, ppv); > +} Using CoCreateInstance is very questionable here (eg. due to the comment above). Also, a patch adding a stub should just return E_NOTIMPL. Implementation deserves a separated patch. I'd expect that it should just use internal ieframe functions to create a new IE instance. Also, I'd suggest to reorder patches to something like: 2, 4, 5, 1 (likely replaced by support for -startmanager command line argument), 3 and then a new patch implementing CreateObject. Thanks, Jacek

2 6

Re: [v3 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 10 Aug '17

10 Aug '17

On 10.08.2017 08:45, Nikolay Sivov wrote: > Based on patch by Anton Romanov <theli.ua(a)gmail.com> > > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 6 ++++-- > dlls/dwrite/font.c | 24 ++++++++++++--------- > dlls/dwrite/main.c | 50 +++++++++++++++++++++++++++++++++----------- > 3 files changed, 56 insertions(+), 24 deletions(-) > Isn't it also necessary to protect factory_cache_fontface()? By the way: An alternative method to implement proper synchronization (which does not require locking critical sections before InterlockedDecrement) would be to do something like: In the destructor: --- snip --- if (!ref) { EnterCriticalSection(...); list_remove(...); LeaveCriticalSection(...); ... } --- snip --- And when querying cached font faces: --- snip --- EnterCriticalSection(...); LIST_FOR_EACH_ENTRY(...) { ... /* the following code could also be hidden in QueryInterface */ if (InterlockedIncrement( &obj->refcount ) == 1) { InterlockedDecrement( &obj->refcount ); continue; } /* return it */ } LeaveCriticalSection(...); --- snip --- This is also what is done in Wines threadpool implementation [1], and has the advantage that the CS is not needed for every Release, only when the object is really destroyed. But in the end it is a matter of taste, for rpcrt4 Jacek also picked the "hold CS during InterlockedDecrement" method. [1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/threadpool.c#l… Best regards, Sebastian

3 2

Re: [PATCH] gdiplus: Support GdipSetClipRegion in metafiles
by Vincent Povirk 10 Aug '17

10 Aug '17

+ stat = GdipGetRegionData(region, (BYTE*)object_record + FIELD_OFFSET(EmfPlusObject, Header.Size), size, NULL); + object_record->Header.DataSize = object_record->Header.Size; + object_record->Header.Size = record_size; This is a little too clever. I'd rather have a solution similar to write_path_data.

1 0

Re: [v4 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 10 Aug '17

10 Aug '17

On 10.08.2017 15:41, Nikolay Sivov wrote: > @@ -861,6 +877,10 @@ HRESULT factory_get_cached_fontface(IDWriteFactory5 *iface, IDWriteFontFile * co > const void *cached_key; > IDWriteFontFile *file; > > + if (IDWriteFontFace4_AddRef(cached->fontface) == 1) > + continue; > + IDWriteFontFace4_Release(cached->fontface); This will not work as expected. The refcount also matters when the object is about to be destroyed. There could be multiple threads running factory_get_cached_fontface() before _Release() can finally remove the fontface from the list. Please note that always calling _Release() also wouldn't be correct because it would run the destructor twice. It is not really necessary to access the private structs from other parts of dwrite, but you should either: * change AddRef to use something like interlocked_dec_if_nonzero (see ntdll/sync.c) * implement the check in QueryInterface (manually run InterlockedDecrement to decrement the refcount without going through the destructor) There are probably also other ways, for example using a flag to make sure the destructor is not called twice. Best regards, Sebastian

1 0

Re: [v2 PATCH 2/2] dwrite: Added a helper to check for supported characters
by Dmitry Timoshkov 10 Aug '17

10 Aug '17

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > - TRACE("(%p)->(0x%08x)\n", This, ch); > + TRACE("(%p)->(0x%#x)\n", This, ch); This will cause printing of the 0x prefix twice. Same applies to other places. -- Dmitry.

1 0

Re: wineandroid: Add suffix to lib directory and wine binary on 64-bit
by Alexandre Julliard 10 Aug '17

10 Aug '17

André Hentschel <nerv(a)dawncrow.de> writes: > This fixes at least arm64-v8a builds, where you have lib64/libwine.so and bin/wine64 > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > > This and "winebuild: Improved position independent code generation" enable Wine on ARM64 Android phones (e.g. most Android devices). > The check for 64 in the CPU_ABI can be discussed, but I think it's reliable and fast. > > dlls/wineandroid.drv/WineActivity.java | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) We already prefix with the ABI, so I think plain "lib" is better. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH v2 2/5] include: Add initial ieautomation.idl
by Nikolay Sivov 09 Aug '17

09 Aug '17

On 08.08.2017 21:04, Zebediah Figura wrote: > +[ > + object, > + uuid(acc84351-04ff-44f9-b23f-655ed168c6d5), > + oleautomation > +] > +interface IInternetExplorerManager : IUnknown > +{ > + HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > +} This is not how it's defined in SDK - oleautomation is not used there and "url" argument has different attributes.

3 2

Re: [PATCH v2 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Alistair Leslie-Hughes 09 Aug '17

09 Aug '17

Hi Zebediah, On 09/08/17 04:04, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > interface IInternetExplorerManager : IUnknown > { > - HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [local] HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [call_as(CreateObject)] HRESULT RemoteCreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); The public API doesn't have the RemoteCreateObject function. Best Regards Alistair.

2 1

Re: [PATCH 3/5] setupapi: Fix compiler warnings when converting between pointers and integers on 64-bit systems
by Alexandre Julliard 09 Aug '17

09 Aug '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/setupapi/devinst.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) That's only hiding the bug. -- Alexandre Julliard julliard(a)winehq.org

4 3

Re: [PATCH v2 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Alistair Leslie-Hughes 09 Aug '17

09 Aug '17

Hi Zebediah, On 09/08/17 04:04, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > interface IInternetExplorerManager : IUnknown > { > - HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [local] HRESULT CreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); > + [call_as(CreateObject)] HRESULT RemoteCreateObject([in] DWORD config, [in, unique] LPCWSTR url, [in] REFIID riid, [out, iid_is(riid)] void **ppv); The public API doesn't have the RemoteCreateObject function. Best Regards Alistair.

1 0

Re: [PATCH] kernel32: Uncomment Win7 stubs in specfile.
by Sebastian Lackner 08 Aug '17

08 Aug '17

On 08.08.2017 21:38, Zebediah Figura wrote: > These were originally commented since the default Windows version > was still XP. As I see you want to increase your regression count. ;) Whats the point of advertising all those functions when Wine has no implementation yet?

3 3

Re: [PATCH 5/5] ieproxy: Add proxy/stub for IInternetExplorerManager.
by Marvin 08 Aug '17

08 Aug '17

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=32620 Your paranoid android. === build (build) === Make in include dir failed

1 0

Re: shell32: Wait for process to finish when using flag SEE_MASK_NOASYNC
by Huw Davies 08 Aug '17

08 Aug '17

On 8 Aug 2017, at 16:50, Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> wrote: > > Support for SEE_MASK_NOASYNC (same value as SEE_MASK_FLAG_DDEWAIT). > Waits for process execution to finish before returning. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/shell32/shlexec.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/dlls/shell32/shlexec.c b/dlls/shell32/shlexec.c > index c0ef53ab8e7..fe3eeeb5743 100644 > --- a/dlls/shell32/shlexec.c > +++ b/dlls/shell32/shlexec.c > @@ -344,6 +344,11 @@ static UINT_PTR SHELL_ExecuteW(const WCHAR *lpCmd, WCHAR *env, BOOL shWait, > dwCreationFlags = CREATE_UNICODE_ENVIRONMENT; > if (!(psei->fMask & SEE_MASK_NO_CONSOLE)) > dwCreationFlags |= CREATE_NEW_CONSOLE; > + > + if(psei->fMask & SEE_MASK_NOASYNC) { > + dwCreationFlags |= SEE_MASK_NOASYNC; > + } > + > if (CreateProcessW(NULL, (LPWSTR)lpCmd, NULL, NULL, FALSE, dwCreationFlags, env, > lpDirectory, &startup, &info)); Passing SEE_MASK_NOASYNC to CreateProcessW doesn’t look right… Huw.

2 2

Invitations to WineConf
by Jeremy White 08 Aug '17

08 Aug '17

Hey folks, Last year we said that we were going to try to broaden our invitations to the Wine conference to try to draw in more voices. This means potentially asking users, and making personal appeals to people that we would like to have at the conference. I plan to write a few personal notes extending invitations to folks that I've thought of (or have had suggested to me). James in our office is also going to reach out. But I'd like to ask everyone to help with that process. Roseanne, I'm particularly hoping you can reach out to the wine users in some way. A post on the forums? Could we / should we do a big news item so it's on the main page? I've asked PlayOnLinux folks as well as the Wineskin folks to come. Anyone else? Cheers, Jeremy

6 6

Re: [v2 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 08 Aug '17

08 Aug '17

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=32619 Your paranoid android. === w2003std (32 bit module) === module.c:471: Test failed: Could not load test library testFlags.dll from C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\ module.c:496: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:505: Test failed: Library testFlags.dll was expected to be loaded from 'system32'

1 0

Re: [PATCH] netapi32: Return ERROR_SUCCESS in DsGetDcNameA/W.
by Huw Davies 08 Aug '17

08 Aug '17

On Fri, Aug 04, 2017 at 04:17:14PM +0300, Andrey Gusev wrote: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > dlls/netapi32/netapi32.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/netapi32/netapi32.c b/dlls/netapi32/netapi32.c > index 278d4528b0..0236a2ad53 100644 > --- a/dlls/netapi32/netapi32.c > +++ b/dlls/netapi32/netapi32.c > @@ -3024,7 +3024,7 @@ DWORD WINAPI DsGetDcNameW(LPCWSTR ComputerName, LPCWSTR AvoidDCName, > FIXME("(%s, %s, %s, %s, %08x, %p): stub\n", debugstr_w(ComputerName), > debugstr_w(AvoidDCName), debugstr_guid(DomainGuid), > debugstr_w(SiteName), Flags, DomainControllerInfo); > - return ERROR_CALL_NOT_IMPLEMENTED; > + return ERROR_SUCCESS; > } > > DWORD WINAPI DsGetDcNameA(LPCSTR ComputerName, LPCSTR AvoidDCName, > @@ -3034,7 +3034,7 @@ DWORD WINAPI DsGetDcNameA(LPCSTR ComputerName, LPCSTR AvoidDCName, > FIXME("(%s, %s, %s, %s, %08x, %p): stub\n", debugstr_a(ComputerName), > debugstr_a(AvoidDCName), debugstr_guid(DomainGuid), > debugstr_a(SiteName), Flags, DomainControllerInfo); > - return ERROR_CALL_NOT_IMPLEMENTED; > + return ERROR_SUCCESS; > } This can't be right. These functions are supposed to return a ptr to an allocated DOMAIN_CONTROLLER_INFO in the final parameter. If you return ERROR_SUCCESS, the app will likely dereference that uninitialized pointer. Huw.

1 0

Re: [PATCH v3] shell32: Implement SHCreateItemInKnownFolder.
by Alistair Leslie-Hughes 08 Aug '17

08 Aug '17

Hi Jactry, On 08/08/17 17:04, Jactry Zeng wrote: > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder || pSHGetKnownFolderPath) If one of these is NULL then the test will crash. Best Regards Alistair

2 1

Re: [PATCH v3] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 08 Aug '17

08 Aug '17

On Tue, Aug 08, 2017 at 03:04:21PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c > index 1e19f7c72e..0bfb4a7f6f 100644 > --- a/dlls/shell32/shellitem.c > +++ b/dlls/shell32/shellitem.c > @@ -662,6 +662,67 @@ HRESULT WINAPI SHCreateItemFromIDList(PCIDLIST_ABSOLUTE pidl, REFIID riid, void > return ret; > } > > +HRESULT WINAPI SHCreateItemInKnownFolder(REFKNOWNFOLDERID rfid, DWORD flags, > + LPWSTR filename, REFIID riid, void **ppv) > +{ > + HRESULT hr; > + WCHAR *pathW = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + > + TRACE("%p, %x, %s, %s, %p\n", rfid, flags, wine_dbgstr_w(filename), > + debugstr_guid(riid), ppv); > + > + if(!rfid || !ppv) > + return E_INVALIDARG; > + > + if(flags) > + FIXME("flags 0x%08x not supported\n", flags); > + > + *ppv = NULL; > + hr = SHGetKnownFolderIDList(rfid, 0, NULL, &pidl_folder); flags should just be passed as the second parameter here and then you can ditch the FIXME(). > + if(hr != S_OK) > + goto cleanup; > + > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) > + goto cleanup; > + > + if(!IsEqualIID(rfid, &FOLDERID_Desktop)) > + { > + hr = IShellFolder_BindToObject(desktop, pidl_folder, NULL, &IID_IShellFolder, > + (void**)&folder); > + if(hr != S_OK) > + goto cleanup; > + } > + > + if(filename) > + { > + hr = IShellFolder_ParseDisplayName(folder ? folder : desktop, NULL, NULL, > + filename, NULL, &pidl, NULL); > + if(hr != S_OK) > + goto cleanup; > + hr = SHCreateItemFromIDList(pidl, riid, ppv); > + if(hr != S_OK) > + goto cleanup; > + } > + else > + hr = SHCreateItemFromIDList(pidl_folder, riid, ppv); Thinking about this some more, how about implementing SHCreateItemFromRelativeName() first? Then this function becomes something like: SHGetKnownFolderIDList( rfid, flags, NULL, &pidl ); SHCreateItemFromIDList( pidl, &IID_IShellItem, &parent ); if (name) SHCreateItemFromRelativeName( parent, name, NULL, iid, ppv ); else IShellItem_QueryInterface( parent, iid, ppv ); IShellItem_Release( parent ); ILFree( pidl ); > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..461f8cea95 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder || pSHGetKnownFolderPath) I think you mean && here (although in practice it doesn't matter). > + if(0) > + { > + /* crashes on Windows */ > + pSHCreateItemInKnownFolder(NULL, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, NULL); > + } I'm not a big fan of this sort of thing, just leave it out. Huw.

1 0

Re: [PATCH v2] shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 08 Aug '17

08 Aug '17

On Fri, Aug 04, 2017 at 02:54:15PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c > index 9e7127dfea..b77619d428 100644 > --- a/dlls/shell32/tests/shlfolder.c > +++ b/dlls/shell32/tests/shlfolder.c > @@ -2519,6 +2521,120 @@ static void test_SHCreateShellItem(void) > else > win_skip("No SHCreateItemFromIDList\n"); > > + /* SHCreateItemInKnownFolder */ > + if(pSHCreateItemInKnownFolder) > + { > + WCHAR *desktop_path; > + WCHAR testfile_path[MAX_PATH] = {0}; > + HANDLE file; > + WCHAR *displayname = NULL; > + int order; > + LPITEMIDLIST pidl_desktop_testfile = NULL; > + > + if(0) > + { > + /* crashes on Windows */ > + pSHCreateItemInKnownFolder(NULL, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, NULL, NULL); > + pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, NULL); > + } > + > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == S_OK, "SHCreateItemInKnownFolder failed: 0x%08x.\n", ret); > + ok(shellitem != NULL, "shellitem was %p.\n", shellitem); > + if(SUCCEEDED(ret)) > + { > + shellitem2 = (void*)0xdeadbeef; > + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem2); > + ok(SUCCEEDED(ret), "SHCreateShellItem returned %x\n", ret); > + if(SUCCEEDED(ret)) > + { > + ret = IShellItem_Compare(shellitem, shellitem2, 0, &order); > + ok(ret == S_OK, "IShellItem_Compare failed: 0x%08x\n", ret); > + ok(!order, "order got wrong value: %d\n", order); > + IShellItem_Release(shellitem2); > + } > + IShellItem_Release(shellitem); > + } > + > + /* Test with a non-existent file */ > + shellitem = (void*)0xdeadbeef; > + ret = pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, > + (void**)&shellitem); > + ok(ret == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Expected 0x%08x but SHCreateItemInKnownFolder return: 0x%08x.\n", > + HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), ret); > + ok(shellitem == NULL, "shellitem was %p.\n", shellitem); > + > + SHGetKnownFolderPath(&FOLDERID_Desktop, 0, NULL, &desktop_path); It looks like SHGetKnownFolderPath is not available on WinXP, so you'll have to load it dynamically. OTOH I think SHCreateItemInKnownFolder() is available on all the platforms that we care about, so you can static link to that one. Huw.

2 1

Re: [PATCH v2] ntdll: Fix wrong string size alloc when loading libraries
by Nikolay Sivov 07 Aug '17

07 Aug '17

On 07.08.2017 20:41, Carlos Palminha wrote: > Fixed some nasty unhandled exceptions when loading libraries > that don't have .dll extension. > > Problem was caused by wrong string size calculation. > > v2: Now properly takes into account strlen * sizeof WCHAR > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/ntdll/loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c > index dd1f74c0fc..83dd6d378e 100644 > --- a/dlls/ntdll/loader.c > +++ b/dlls/ntdll/loader.c > @@ -2141,7 +2141,7 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, > if (!(ext = strrchrW( libname, '.')) || strchrW( ext, '/' ) || strchrW( ext, '\\')) > { > if (!(dllname = RtlAllocateHeap( GetProcessHeap(), 0, > - (strlenW(libname) * sizeof(WCHAR)) + sizeof(dllW) ))) > + (strlenW(libname) * sizeof(WCHAR)) + (strlenW(dllW) * sizeof(WCHAR)) ))) > return STATUS_NO_MEMORY; > strcpyW( dllname, libname ); > strcatW( dllname, dllW ); > This is still shorter than it is now: sizeof(dllW) is 5 * sizeof(WCHAR) vs strlenW(dllW) * sizeof(WCHAR) == 4 * sizeof(WCHAR).

2 1

Re: [PATCH] ws2_32: Don't return synchronously if the connection is refused.
by Huw Davies 07 Aug '17

07 Aug '17

On Mon, Aug 07, 2017 at 03:39:14PM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/ws2_32/socket.c | 5 +++++ > dlls/ws2_32/tests/sock.c | 20 ++++++-------------- > 2 files changed, 11 insertions(+), 14 deletions(-) Please ignore this for now, it needs more work. Huw.

1 0

Re: [PATCH] ntdll: Fix wrong string size alloc when loading libraries
by Józef Kucia 07 Aug '17

07 Aug '17

On Mon, Aug 7, 2017 at 6:48 PM, Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> wrote: > Fixed some nasty unhandled exceptions when loading libraries > that don't have .dll extension. > > Problem was caused by wrong string size calculation. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > dlls/ntdll/loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c > index dd1f74c0fc..8fc464bcd7 100644 > --- a/dlls/ntdll/loader.c > +++ b/dlls/ntdll/loader.c > @@ -2141,7 +2141,7 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, > if (!(ext = strrchrW( libname, '.')) || strchrW( ext, '/' ) || strchrW( ext, '\\')) > { > if (!(dllname = RtlAllocateHeap( GetProcessHeap(), 0, > - (strlenW(libname) * sizeof(WCHAR)) + sizeof(dllW) ))) > + (strlenW(libname) * sizeof(WCHAR)) + strlenW(dllW) ))) This doesn't look right. After the change it allocates less memory than before. Notice that strlenW(dllW) == 4 and sizeof(dllW) == 10. > return STATUS_NO_MEMORY; > strcpyW( dllname, libname ); > strcatW( dllname, dllW ); > -- > 2.11.0 > > >

2 1

Re: [PATCH 2/2] wined3d: do not call queue_is_empty in wined3d_cs_mt_finish.
by Henri Verbeet 07 Aug '17

07 Aug '17

On 4 August 2017 at 10:21, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > +/* This function assumes that it is called from the command stream thread. */ > static BOOL wined3d_cs_queue_is_empty(const struct wined3d_cs_queue *queue) It should probably have a wined3d_from_cs() then.

1 0

Re: winebuild: Improved position independent code generation
by Martin Storsjö 06 Aug '17

06 Aug '17

On Sun, 6 Aug 2017, André Hentschel wrote: > Android only accepts proper position independent binaries, but until now we generated code which led to a TEXTREL flag > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > tools/winebuild/import.c | 34 ++++++++++++++-------------------- > tools/winebuild/spec32.c | 4 ++-- > 2 files changed, 16 insertions(+), 22 deletions(-) Signed-off-by: Martin Storsjo <martin(a)martin.st> // Martin

1 0

Re: [PATCH] msvcrt: Fix strcpy implementation so it works on overlapping buffers
by Ken Thomases 05 Aug '17

05 Aug '17

On Aug 5, 2017, at 10:43 AM, Piotr Caban <piotr(a)codeweavers.com> wrote: > +char* CDECL MSVCRT_strcpy(char *dst, const char *src) > +{ > + char *ret = dst; > + > + while(1) { > + *dst = *src; > + if (!*src) break; > + src++; > + *dst++; What's the dereference for? > + } > + > + return ret; > +} -Ken

2 1

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 3]
by Vincent Povirk 04 Aug '17

04 Aug '17

I think it would make more sense to do the initial random delay on the background threads.

2 2

Re: shell32: Implement SHCreateItemInKnownFolder.
by Huw Davies 04 Aug '17

04 Aug '17

On Sun, Jul 30, 2017 at 05:37:02PM +0800, Jactry Zeng wrote: > diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c > index 1e19f7c72e..57fb849f16 100644 > --- a/dlls/shell32/shellitem.c > +++ b/dlls/shell32/shellitem.c > @@ -662,6 +662,66 @@ HRESULT WINAPI SHCreateItemFromIDList(PCIDLIST_ABSOLUTE pidl, REFIID riid, void > return ret; > } > > +HRESULT WINAPI SHCreateItemInKnownFolder(REFKNOWNFOLDERID rfid, DWORD flags, > + LPWSTR filename, REFIID riid, void **ppv) > +{ > + HRESULT hr; > + WCHAR *pathW = NULL; > + IShellFolder *desktop = NULL, *folder = NULL; > + LPITEMIDLIST pidl_folder = NULL, pidl = NULL; > + > + TRACE("%p, %x, %s, %s, %p\n", rfid, flags, wine_dbgstr_w(filename), > + debugstr_guid(riid), ppv); > + > + if(!rfid || !ppv) > + return E_INVALIDARG; > + > + if(flags) > + FIXME("flags 0x%08x not supported\n", flags); > + > + *ppv = NULL; > + hr = SHGetDesktopFolder(&desktop); > + if(hr != S_OK) return hr; > + > + hr = SHGetKnownFolderPath(rfid, 0, NULL, &pathW); > + if(hr != S_OK) return hr; > + > + hr = IShellFolder_ParseDisplayName(desktop, NULL, NULL, pathW, NULL, &pidl_folder, NULL); > + if(hr != S_OK) > + goto cleanup; You probably want to use SHGetKnownFolderIDList() instead of these last two calls. Huw.

2 3

Re: [4/4] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 03 Aug '17

03 Aug '17

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=32575 Your paranoid android. === w2003std (32 bit module) === module.c:443: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x), got 87 module.c:471: Test failed: Could not load test library testFlags.dll from C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\ module.c:472: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 module.c:481: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x), got 87 module.c:496: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:497: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 module.c:505: Test failed: Library testFlags.dll was expected to be loaded from 'system32' module.c:506: Test failed: Expected 0xdeadbeef or ERROR_SUCCESS, got 87 === w7u (32 bit loader) === loader.c:2199: Test failed: expected WAIT_OBJECT_0, got 0x102 loader.c:2209: Test failed: expected thread exit code 196, got 259 loader.c:1713: Test failed: expected thread exit code 196, got 195

1 0

Re: [PATCH 2/2] winebrowser: Open the default browser if called without an argument.
by Alexandre Julliard 03 Aug '17

03 Aug '17

Zebediah Figura <zfigura(a)codeweavers.com> writes: > xdg-open can't handle this, so work around with the following command > (suggested by Vincent Povirk): > gtk-launch `xdg-mime query default x-scheme-handler/http` > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > programs/winebrowser/main.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) I'd suggest to simply open the default start page in that case. That's what iexplore does when there are no arguments. -- Alexandre Julliard julliard(a)winehq.org

3 5

Re: [1/2] msvcrt/tests: Add tests for mbstowcs and wcstombs with empty strings.
by Piotr Caban 03 Aug '17

03 Aug '17

Hi, the patch causes compilation warnings on 64-bit wine. ../../../../wine_src/dlls/msvcrt/tests/string.c: In function 'test_mbstowcs': ../../../../wine_src/dlls/msvcrt/tests/string.c:1869:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1872:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1887:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1890:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1903:228: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1911:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1932:228: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] ../../../../wine_src/dlls/msvcrt/tests/string.c:1951:123: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=] Thanks, Piotr

1 0

Re: ntdll: Support receiving server reply data to memory with write watch.
by Alexandre Julliard 03 Aug '17

03 Aug '17

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -256,6 +256,7 @@ static void read_reply_data( void *buffer, size_t size ) > if (!ret) break; > if (errno == EINTR) continue; > if (errno == EPIPE) break; > + if (errno == EFAULT && virtual_uninterrupted_write_memory( buffer, NULL, size ) >= size) continue; > server_protocol_perror("read"); You can't enter a critical section in the middle of a server call. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH v3] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Huw Davies 03 Aug '17

03 Aug '17

On Wed, Aug 02, 2017 at 11:00:04AM -0500, Zebediah Figura wrote: > +static void COM_RevokeAllPSClsids(void) > +{ How about calling it revoke_registered_psclsids() ? Otherwise, this looks good. Huw.

1 0

Re: [4/6] wsdapi: Implement "Hello" message creation support. [try 2]
by Vincent Povirk 02 Aug '17

02 Aug '17

You're still using rand/srand in this one.

2 1

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements. [try 2]
by Vincent Povirk 02 Aug '17

02 Aug '17

+static void remove_attribute(WSDXML_ELEMENT *parent, WSDXML_ATTRIBUTE *attribute) +{ + WSDXML_ATTRIBUTE *curAttrib; + + /* Find the last attribute and add this as the next one */ + curAttrib = parent->FirstAttribute; + + if (curAttrib == attribute) + { + parent->FirstAttribute = NULL; + } I think this should be parent->FirstAttribute = curAttrib->Next. + } + else if (curNode->Type == TextType) + { + textNode = WSDAllocateLinkedMemory(newElement, sizeof(WSDXML_TEXT)); + if (textNode == NULL) goto cleanup; + + textNode->Node.Next = NULL; + textNode->Node.Parent = newElement; + textNode->Node.Type = TextType; + textNode->Text = duplicate_string(textNode, ((WSDXML_TEXT *)curNode)->Text); + if (textNode->Text == NULL) goto cleanup; + } You still don't seem to be adding the new text node to newElement.

2 1

Re: [PATCH v2] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Huw Davies 02 Aug '17

02 Aug '17

On Tue, Aug 01, 2017 at 04:54:45PM -0500, Zebediah Figura wrote: > v2: use proper synchronization; add an MTA test (thanks Sebastian) > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/ole32/compobj.c | 48 ++++++++++++++++++++++++++++++------------ > dlls/ole32/compobj_private.h | 1 - > dlls/ole32/tests/compobj.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 83 insertions(+), 16 deletions(-) > > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c > index 60244485249..ee904adbc38 100644 > --- a/dlls/ole32/compobj.c > +++ b/dlls/ole32/compobj.c > @@ -170,8 +170,20 @@ struct registered_psclsid > struct list entry; > IID iid; > CLSID clsid; > + OXID apartment_id; > }; > > +static struct list RegisteredPSCLSIDList = LIST_INIT(RegisteredPSCLSIDList); Please don't use CamelCase. > + > +static CRITICAL_SECTION csRegisteredPSCLSIDList; > +static CRITICAL_SECTION_DEBUG psclsid_cs_debug = > +{ > + 0, 0, &csRegisteredPSCLSIDList, > + { &psclsid_cs_debug.ProcessLocksList, &psclsid_cs_debug.ProcessLocksList }, > + 0, 0, { (DWORD_PTR)(__FILE__ ": csRegisteredPSCLSIDList") } > +}; > +static CRITICAL_SECTION csRegisteredPSCLSIDList = { &psclsid_cs_debug, -1, 0, 0, 0, 0 }; > + > /* > * This is a marshallable object exposing registered local servers. > * IServiceProvider is used only because it happens meet requirements > @@ -622,7 +634,6 @@ static APARTMENT *apartment_construct(DWORD model) > > list_init(&apt->proxies); > list_init(&apt->stubmgrs); > - list_init(&apt->psclsids); > list_init(&apt->loaded_dlls); > apt->ipidc = 0; > apt->refs = 1; > @@ -1173,15 +1184,22 @@ DWORD apartment_release(struct apartment *apt) > stub_manager_int_release(stubmgr); > } > > - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids) > + EnterCriticalSection(&csRegisteredPSCLSIDList); > + > + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList) > { > struct registered_psclsid *registered_psclsid = > LIST_ENTRY(cursor, struct registered_psclsid, entry); > > - list_remove(&registered_psclsid->entry); > - HeapFree(GetProcessHeap(), 0, registered_psclsid); > + if (registered_psclsid->apartment_id == apt->oxid) > + { > + list_remove(&registered_psclsid->entry); > + HeapFree(GetProcessHeap(), 0, registered_psclsid); > + } > } > > + LeaveCriticalSection(&csRegisteredPSCLSIDList); > + > /* if this assert fires, then another thread took a reference to a > * stub manager without taking a reference to the containing > * apartment, which it must do. */ It seems it's more complicated than this. Consider two already existing STAs, if the PSClsid is registered in STA1 which then leaves the apt, a call in STA2 to CoGetPSCLSID() still succeeds. You'll want to expand the tests to cover things like this to really figure out what's going on. Also note that the MTA -> MTA test isn't very convincing, since there is only one MTA and hence both threads are in the same apt. Huw.

2 1

Re: [PATCHv2] msvcrt: Export -f suffixed float functions functions on arm64
by Piotr Caban 02 Aug '17

02 Aug '17

Hi Martin, On 07/31/17 14:47, Martin Storsjo wrote: > --- a/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec > +++ b/dlls/api-ms-win-crt-math-l1-1-0/api-ms-win-crt-math-l1-1-0.spec > @@ -77,7 +77,7 @@ > @ cdecl _fdtest(ptr) ucrtbase._fdtest > @ stub _fdunscale > @ cdecl _finite(double) ucrtbase._finite > -@ cdecl -arch=arm,x86_64 _finitef(float) ucrtbase._finitef > +@ cdecl -arch=arm,x86_64,arm64 _finitef(float) ucrtbase._finitef > @ cdecl _fpclass(double) ucrtbase._fpclass > @ stub _fpclassf > @ cdecl -arch=i386 -ret64 _ftol() ucrtbase._ftol > @@ -85,7 +85,7 @@ > @ cdecl _hypot(double double) ucrtbase._hypot > @ cdecl _hypotf(float float) ucrtbase._hypotf > @ cdecl _isnan(double) ucrtbase._isnan > -@ cdecl -arch=arm,x86_64 _isnanf(float) ucrtbase._isnanf > +@ cdecl -arch=arm,x86_64,arm64 _isnanf(float) ucrtbase._isnanf Maybe I'm checking it incorrectly but I don't see isnanf in the lib file. Is it really exported on arm64? I didn't check all of the functions but _scalbf also seems to be not exported. Thanks, Piotr

2 1

Re: [PATCH 2/3] msvcrt: Set errno through the _matherr function.
by Piotr Caban 02 Aug '17

02 Aug '17

On 08/01/17 08:13, Alex Henrie wrote: > @@ -252,7 +320,7 @@ float CDECL MSVCRT_powf( float x, float y ) > { > /* FIXME: If x < 0 and y is not integral, set EDOM */ > float z = powf(x,y); > - if (!finitef(z)) *MSVCRT__errno() = MSVCRT_EDOM; > + if (!finitef(z)) math_error(_DOMAIN, "powf", x, 0, ret); ret is not defined here. Could you please also add exception.name tests (in this case it should be set to pow instead of powf). Thanks, Piotr

2 2

Re: [5/6] wsdapi: Add support for sending messages via UDP multicast.
by Vincent Povirk 01 Aug '17

01 Aug '17

+ delay = rand() % (UDP_MAX_DELAY - UDP_MIN_DELAY + 1) + UDP_MIN_DELAY; Using rand() for this doesn't seem very random, and it could interfere with an application's use of rand(). + /* Get size of buffer for adapters */ + adapterAddresses = (IP_ADAPTER_ADDRESSES *) HeapAlloc(heap, 0, bufferSize); Was this comment meant to be earlier? Sending messages to every adapter in sequence, with a delay for each, doesn't seem right. +void terminate_networking(IWSDiscoveryPublisherImpl *impl) +{ + impl->publisherStarted = FALSE; + + WSACleanup(); +} It seems like this will call WSACleanup even when freeing an object that was never used.

2 1

Re: [PATCH] ole32: Store proxy/stub CLSIDs per process, not per apartment.
by Sebastian Lackner 01 Aug '17

01 Aug '17

On 01.08.2017 23:27, Zebediah Figura wrote: > @@ -1173,13 +1175,16 @@ DWORD apartment_release(struct apartment *apt) > stub_manager_int_release(stubmgr); > } > > - LIST_FOR_EACH_SAFE(cursor, cursor2, &apt->psclsids) > + LIST_FOR_EACH_SAFE(cursor, cursor2, &RegisteredPSCLSIDList) Accessing a global list from multiple threads requires synchronization (for example, using a CS to avoid conflicting access). Using apt->cs is not sufficient because it might be a different one for each thread. The same also applies to all other places where the list is used. > @@ -1177,6 +1196,15 @@ static void test_CoRegisterPSClsid(void) > hr = CoRegisterPSClsid(&IID_IWineTest, &CLSID_WineTestPSFactoryBuffer); > ok_ole_success(hr, "CoRegisterPSClsid"); > > + hr = CoGetPSClsid(&IID_IWineTest, &clsid); > + ok_ole_success(hr, "CoGetPSClsid"); > + ok(IsEqualGUID(&clsid, &CLSID_WineTestPSFactoryBuffer), "expected %s, got %s\n", > + wine_dbgstr_guid(&CLSID_WineTestPSFactoryBuffer), wine_dbgstr_guid(&clsid)); > + > + thread = CreateThread(NULL, 0, register_ps_clsid_thread, NULL, 0, &tid); > + ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); > + ok(!WaitForSingleObject(thread, 10000), "wait timed out\n"); The thread handle is leaked here. Also, it would be more convincing to run the tests for both STA and MTA.

1 0

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements.
by Vincent Povirk 01 Aug '17

01 Aug '17

+ wideStringLength = lstrlenW(wideString); + sizeNeeded = WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, NULL, 0, NULL, NULL); + + if (sizeNeeded < 0) + { + return NULL; + } + + newString = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeNeeded + 1); + WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, newString, sizeNeeded, NULL, NULL); This would be much simpler if you passed in -1 as the wide string length. Using HEAP_ZERO_MEMORY to null-terminate the string is a bit weird. + else if (curNode->Type == TextType) + { + textNode = WSDAllocateLinkedMemory(parent, sizeof(WSDXML_TEXT)); + if (textNode == NULL) goto cleanup; + + textNode->Node.Next = NULL; + textNode->Node.Parent = parent; + textNode->Node.Type = TextType; + textNode->Text = duplicate_string(textNode, ((WSDXML_TEXT *)curNode)->Text); + if (textNode->Text == NULL) goto cleanup; + } I'm not sure, but I think you need to add textNode as a child of newElement? + /* SequenceID attribute */ + if (header->AppSequence->SequenceId != NULL) + { + if (!add_string_attribute(xmlContext, appSequenceElement, discoveryNsUri, sequenceIdString, header->AppSequence->SequenceId)) goto cleanup; + } Do we need the namespace for SequenceID?

2 3

Re: [PATCH 1/2] winebrowser: Try other browsers on non-zero exit code.
by Vincent Povirk 01 Aug '17

01 Aug '17

Using system is usually a bad idea for security reasons. I also don't think waiting for the browser command to return makes sense. It may keep winebrowser running longer than expected, and we don't know for sure what the return value means.

2 1

Re: [PATCH 5/6 v3] ws2_32: Added support for overlapped GetAddrInfoExW.
by Sebastian Lackner 01 Aug '17

01 Aug '17

On 01.08.2017 19:08, Jacek Caban wrote: > + > + overlapped->Internal = ret; > + if (overlapped->hEvent) SetEvent(overlapped->hEvent); Not sure how it is implemented on Windows, but caching overlapped->hEvent would ensure that it also works when the overlapped struct is immediately reused after ->Internal changed.

1 0

Re: [PATCH 5/6] ws2_32: Added support for overlapped GetAddrInfoExW.
by Sebastian Lackner 01 Aug '17

01 Aug '17

On 01.08.2017 18:29, Jacek Caban wrote: > +static void WINAPI getaddrinfo_callback(TP_CALLBACK_INSTANCE *instance, void *context, TP_WORK *work) > +{ > + struct getaddrinfo_args *args = context; > + OVERLAPPED *overlapped = args->overlapped; > + struct WS_addrinfo *res; > + > + overlapped->Internal = WS_getaddrinfo(args->nodename, args->servname, NULL, &res); What if the app does busy-looping instead of using the event, and expects the result to be set after overlapped->Internal changed? My understanding is that setting overlapped->Internal should be the very last thing in the function, the app could decide to deallocate / reuse the memory after the async operation has finished. > + if (res) > + { > + *args->result = addrinfo_list_AtoW(res); > + overlapped->u.Pointer = args->result; > + WS_freeaddrinfo(res); > + } >[...] > + tp_work = CreateThreadpoolWork(getaddrinfo_callback, args, NULL); > + if (!tp_work) > + { > + HeapFree(GetProcessHeap(), 0, args); > + ret = GetLastError(); > + goto end; > + } > > + overlapped->Internal = WSAEINPROGRESS; > + SubmitThreadpoolWork(tp_work); I don't see a CloseThreadpoolWork call or cleanup group anywhere, so there is likely a leak in this code. Also, when all callbacks have different arguments, it would be better to use TrySubmitThreadpoolCallback(). Work callbacks are typically used when the same task is queued multiple times. > + if (local_nodenameW != nodename) > + HeapFree(GetProcessHeap(), 0, local_nodenameW); > + WSASetLastError(ERROR_IO_PENDING); > + return ERROR_IO_PENDING; > + } > + > + ret = WS_getaddrinfo(nodenameA, servnameA, hints, &resA);

2 1

Re: [PATCH 6/6] ws2_32/tests: Added GetAddrInfoExW tests.
by Alexandre Julliard 01 Aug '17

01 Aug '17

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -7334,6 +7343,95 @@ static void test_GetAddrInfoW(void) > ok(result2 == NULL, "got %p\n", result2); > } > > +static void test_GetAddrInfoExW(void) > +{ You are not calling this anywhere. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alexandre Julliard 01 Aug '17

01 Aug '17

Zebediah Figura <zfigura(a)codeweavers.com> writes: > +[ > + object, > + uuid(1a3114b8-a62e-11d0-a6c5-00a0c906af45), > + local > +] > +interface IPersistQuery : IPersist > +{ > + HRESULT Clear(); > + > + HRESULT ReadInt([in] LPCWSTR section, [in] LPCWSTR name, [out] INT *value); > + > + HRESULT ReadString([in] LPCWSTR section, [in] LPCWSTR name, [out, size_is(buflen)] LPWSTR buffer, [in] INT buflen); > + > + HRESULT ReadStruct([in] LPCWSTR section, [in] LPCWSTR name, [out, size_is(buflen)] LPVOID buffer, [in] DWORD buflen); > + > + HRESULT WriteInt([in] LPCWSTR section, [in] LPCWSTR name, [in] INT value); > + > + HRESULT WriteString([in] LPCWSTR section, [in] LPCWSTR name, [in] LPCWSTR value); > + > + HRESULT WriteStruct([in] LPCWSTR section, [in] LPCWSTR name, [in] LPVOID value, [in] DWORD size); > +}; This doesn't match the PSDK, the order matters. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alistair Leslie-Hughes 01 Aug '17

01 Aug '17

Hi Zebediah, On 01/08/17 05:24, Zebediah Figura wrote: > v2: fix a typo in MSDN > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > +cpp_quote("DEFINE_GUID(CLSID_CommonQuery, 0x83BC5EC0, 0x6F2A, 0x11D0, 0xA1,0xC4, 0x00,0xAA,0x00,0xC1,0x6E,0x65);") > Please make the GUID lowercase. Best Regards Alistair

1 0

Re: [PATCH v2 2/3] include: Add initial IDL for cmnquery.h
by Alistair Leslie-Hughes 01 Aug '17

01 Aug '17

Hi Zebediah, On 01/08/17 05:24, Zebediah Figura wrote: > v2: fix a typo in MSDN > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > +cpp_quote("DEFINE_GUID(CLSID_CommonQuery, 0x83BC5EC0, 0x6F2A, 0x11D0, 0xA1,0xC4, 0x00,0xAA,0x00,0xC1,0x6E,0x65);") > Please make the GUID lowercase. Best Regards Alistair

1 0

Re: [PATCH] kernel32: Load library search in system32 directory.
by Carlos Palminha 01 Aug '17

01 Aug '17

On 31-07-2017 18:50, Alexandre Julliard wrote: > Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: > >> On 31-07-2017 17:54, Alexandre Julliard wrote: >>> Carlos Palminha <CARLOS.PALMINHA(a)synopsys.com> writes: >>> >>>> Hi Alexandre, >>>> >>>> I tested with iTunes installation that was crashing when calling load_libraryin my system. After the patch it was ok. Probably not enough indicator that was working properly in all systems. >>>> >>>> I will send a second version calling GetWindowsDirectory instead of using %windows%. >>>> >>>> Regarding the other cases, when using the flag >>>> LOAD_LIBRARY_SEARCH_SYSTEM32, directories in the standard search path >>>> are not searched and the value cannot be combined with >>>> LOAD_WITH_ALTERED_SEARCH_PATH. The search path is reduced to >>>> windows\system32, right? Or probably i need to append >>>> %windows&\system32 to the existing path... >>> >>> If you only specify SEARCH_SYSTEM32 the path is reduced to that, but you >>> could also have other flags like SEARCH_APPLICATION_DIR, >>> SEARCH_DLL_LOAD_DIR, etc. You can't handle just one flag and ignore the >>> others. >>> >> >> All the other cases SEARCH_APPLICATION_DIR, SEARCH_DEFAULT_DIRS, SEARCH_DLL_LOAD_DIR and SEARCH_USER_DIRS are not yet supported (unsopported flags). >> So at this moment LOAD_LIBRARY_SEARCH_* are reduced to ALTERED_SEARCH_PATH or SEARCH_SYSTEM32. > > They are not supported now, but if you add one you have to add the > others, otherwise dlls are going to fail to load because you are > searching only system32. > I think today its working for some of those unsupported flags because it's a side effect of calling MODULE_get_dll_load_path with NULL. Probably using only system32 path it might break some of the application that are working based on a side effect. Nevertheless i run the existing tests for module and its ok. I will look at it and add the other SEARCH flags. If i check the other flags then there is no need to fallback to MODULE_get_dll_load_path(NULL), do you agree? Regards, C.Palminha

2 3

Re: [PATCH 3/3] ucrtbase/tests: Add tests for math function errors.
by Marvin 01 Aug '17

01 Aug '17

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=32534 Your paranoid android. === wvistau64 (64 bit misc) === misc.c:691: Test failed: log2(-1.#INF00) got exception type 1 misc.c:691: Test failed: log2(-1.000000) got exception type 1 misc.c:691: Test failed: log2(0.000000) got exception type 2 misc.c:721: Test failed: _scalb(1.000000, -1000000000) got exception type -1 === w1064 (64 bit misc) === misc.c:691: Test failed: log2(-1.#INF00) got exception type 1 misc.c:691: Test failed: log2(-1.000000) got exception type 1 misc.c:691: Test failed: log2(0.000000) got exception type 2 misc.c:721: Test failed: _scalb(1.000000, -1000000000) got exception type -1

1 0

Re: [PATCH] opengl32: Update OpenGL Registry files URLs.
by Austin English 01 Aug '17

01 Aug '17

On Mon, Jul 31, 2017 at 5:14 PM, Józef Kucia <jkucia(a)codeweavers.com> wrote: > The OpenGL Registry has been moved. See > https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/README.txt. > > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/opengl32/make_opengl | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/dlls/opengl32/make_opengl b/dlls/opengl32/make_opengl > index 45e3f720ea..ef4156686a 100755 > --- a/dlls/opengl32/make_opengl > +++ b/dlls/opengl32/make_opengl > @@ -8,8 +8,8 @@ use XML::Simple; > # > # - It needs files from the OpenGL extension registry: > # > -# https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/gl.xml > -# https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/wgl.xml > +# https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/gl.xml > +# https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/wgl.xml > # > # If they are not found in the current directory the script will > # attempt to download them from there. > @@ -404,8 +404,8 @@ if ($version eq "1.0") { > # > # Fetch the registry files > # > --f "gl.xml" || system "wget --no-check-certificate https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/gl.xml" || die "cannot download gl.xml"; > --f "wgl.xml" || system "wget --no-check-certificate https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/wgl.xml" || die "cannot download wgl.xml"; > +-f "gl.xml" || system "wget --no-check-certificate https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/gl.xml" || die "cannot download gl.xml"; > +-f "wgl.xml" || system "wget --no-check-certificate https://raw.github.com/KhronosGroup/OpenGL-Registry/master/xml/wgl.xml" || die "cannot download wgl.xml"; > > > # > -- > 2.13.0 Do we really want to keep the --no-check-certificate now the file is on github.com? I feel like github's cert is pretty widely trusted, and likely to stay working. Especially since all of *winehq.org is finally on HTTPS.. -- -Austin GPG: 14FB D7EA A041 937B

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel August 2017