September 2017 - Wine-Devel

Re: [PATCH 3/5] wined3d: Advertise AUTOGENMIPMAP only for renderable and filterable formats.
by Marvin Sept. 20, 2017

Sept. 20, 2017

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=33191 Your paranoid android. === w8 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w864 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w1064 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w864 (64 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w1064 (64 bit device) === device.c:6945: Test failed: Got unexpected hr 0.

1 0

Re: [PATCH 2/5] d3d9/tests: Add a test for nonrenderable format D3DUSAGE_AUTOGENMIPMAP support.
by Marvin Sept. 20, 2017

Sept. 20, 2017

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=33190 Your paranoid android. === w8 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w864 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w1064 (32 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w864 (64 bit device) === device.c:6945: Test failed: Got unexpected hr 0. === w1064 (64 bit device) === device.c:6945: Test failed: Got unexpected hr 0.

1 0

Re: [PATCH] GdiPlus: Add GdipCreateLineBrushFromRectWithAngle tests and fix compat
by Vincent Povirk Sept. 20, 2017

Sept. 20, 2017

This causes warnings: ../../../../dlls/gdiplus/tests/brush.c: In function ‘test_createLineBrushFromRectWithAngle’: ../../../../dlls/gdiplus/tests/brush.c:85:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] GpRectF rect2 = { 1, 3, -1, -2 }; ^ ../../../../dlls/gdiplus/tests/brush.c:108:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] GpRectF rect3 = { 1, 3, 0, 1 }; ^ ../../../../dlls/gdiplus/tests/brush.c:112:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] GpRectF rect4 = { 1, 3, 1, 0 }; ^

1 0

Re: [1/3] ole32: Add a trace to IDataObject::GetData.
by Huw Davies Sept. 20, 2017

Sept. 20, 2017

On Wed, Sep 20, 2017 at 02:54:05PM +0300, Dmitry Timoshkov wrote: > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > --- > dlls/ole32/datacache.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c > index ecae89d284..4e684b4189 100644 > --- a/dlls/ole32/datacache.c > +++ b/dlls/ole32/datacache.c > @@ -1245,6 +1245,8 @@ static HRESULT WINAPI DataCache_GetData( > DataCache *This = impl_from_IDataObject(iface); > DataCacheEntry *cache_entry; > > + TRACE("%p,%s,%p\n", iface, debugstr_formatetc(pformatetcIn), pmedium); > + > memset(pmedium, 0, sizeof(*pmedium)); > > cache_entry = DataCache_GetEntryForFormatEtc(This, pformatetcIn); I've resent this one and [2/3] with spaces between the commas in the TRACE()s. I hope that's ok with you. Huw.

2 1

Re: [PATCH] include: Remove D3D_DISABLE_9EX checks.
by Henri Verbeet Sept. 20, 2017

Sept. 20, 2017

On 20 September 2017 at 07:13, Alex Henrie <alexhenrie24(a)gmail.com> wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=37160 > > The other option is to add D3D_DISABLE_9EX checks to d3d9.h, but I don't > think it will break anything to declare the new stuff unconditionally. > I'd rather do it right.

1 0

[PATCH 4/4] include: Add dxgi1_6.idl
by Shinchiro Shinchiro Sept. 20, 2017

Sept. 20, 2017

When I try compiling this idl with widl, I'm getting segfault. I can't figure what's wrong with it

1 1

Re: [PATCH v2 2/2] msi: Store the current script in the package.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 11:59 -0500, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/msi/action.c | 25 +++++++++++++++---------- > dlls/msi/custom.c | 34 ++-------------------------------- > dlls/msi/dialog.c | 2 +- > dlls/msi/install.c | 8 ++++---- > dlls/msi/msi.c | 6 +++--- > dlls/msi/msipriv.h | 8 +++----- > dlls/msi/package.c | 1 + > 7 files changed, 29 insertions(+), 55 deletions(-) action.c: In function ‘execute_script’: action.c:1549:16: warning: assignment makes integer from pointer without a cast for (i = package->script_actions_count; i > 0; i--) ^

1 0

Re: [PATCH v2 1/2] msi: Remove a superfluous substructure.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 11:58 -0500, Zebediah Figura wrote: > v2: don't use lists > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/msi/action.c | 45 ++++++++++++++++++++------------------------- > dlls/msi/custom.c | 39 ++++++++++++++++----------------------- > dlls/msi/msipriv.h | 36 +++++++++++++++--------------------- > dlls/msi/package.c | 31 ++++++++++++------------------- > 4 files changed, 63 insertions(+), 88 deletions(-) This gives a warning: package.c: In function ‘msi_alloc_package’: package.c:999:9: warning: unused variable ‘i’ [-Wunused-variable] int i; ^

1 0

Re: [PATCH 3/3] msi: Remove properties from deferred custom actions.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote: > This effectively reverts 3c444ea350e0688247bd2056b3861dac4af08fbc, > which should not be necessary, and in fact does not affect the > linked application, MSN Messenger. This needs a test.

2 1

Re: [PATCH] Spec entries fixes.
by Alexandre Julliard Sept. 19, 2017

Sept. 19, 2017

Andrey Gusev <andrey.goosev(a)gmail.com> writes: > Signed-off-by: Andrey Gusev <andrey.goosev(a)gmail.com> > --- > dlls/advapi32/advapi32.spec | 2 +- > .../api-ms-win-core-console-l2-1-0.spec | 2 +- > .../api-ms-win-core-localization-l2-1-0.spec | 2 +- > .../api-ms-win-core-registry-l2-1-0.spec | 2 +- > .../ext-ms-win-rtcore-ntuser-sysparams-l1-1-0.spec | 2 +- > dlls/kernel32/kernel32.spec | 4 +-- > dlls/kernelbase/kernelbase.spec | 4 +-- > dlls/mpr/mpr.spec | 2 +- > dlls/rasapi32/rasapi32.spec | 4 +-- > dlls/shlwapi/shlwapi.spec | 4 +-- > dlls/unicows/unicows.spec | 38 +++++++++++----------- > dlls/user32/user32.spec | 6 ++-- > 12 files changed, 36 insertions(+), 36 deletions(-) Please don't do large scale changes like this, it should be one module at a time. Also please try to lookup previous similar patches, a number of these have been rejected already. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/3] msi: Store the current script in the package.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote: > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c > index a3e32d9a24..b463ab659c 100644 > --- a/dlls/msi/custom.c > +++ b/dlls/msi/custom.c > @@ -1123,24 +1123,6 @@ static UINT HANDLE_CustomType53_54( MSIPACKAGE *package, const WCHAR *source, co > return wait_thread_handle( info ); > } > > -static BOOL action_type_matches_script( UINT type, UINT script ) > -{ > - switch (script) > - { > - case SCRIPT_NONE: > - return FALSE; > - case SCRIPT_INSTALL: > - return !(type & msidbCustomActionTypeCommit) && !(type & msidbCustomActionTypeRollback); > - case SCRIPT_COMMIT: > - return (type & msidbCustomActionTypeCommit); > - case SCRIPT_ROLLBACK: > - return (type & msidbCustomActionTypeRollback); > - default: > - ERR("unhandled script %u\n", script); > - } > - return FALSE; > -} > - > static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT type ) > { > WCHAR *actiondata = msi_dup_property( package->db, action ); > @@ -1178,7 +1160,7 @@ static UINT defer_custom_action( MSIPACKAGE *package, const WCHAR *action, UINT > return ERROR_SUCCESS; > } > > -UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script ) > +UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action ) > { > static const WCHAR query[] = { > 'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', > @@ -1218,7 +1200,7 @@ UINT ACTION_CustomAction( MSIPACKAGE *package, LPCWSTR action, UINT script ) > if (type & msidbCustomActionTypeNoImpersonate) > WARN("msidbCustomActionTypeNoImpersonate not handled\n"); > > - if (!action_type_matches_script( type, script )) > + if (package->script == SCRIPT_NONE) > { This function was called with script other than SCRIPT_NONE before, so this changes the order of execution. It would need a test, and should be in a separate patch.

2 2

Re: [PATCH 1/3] msi: Remove a superfluous substructure and convert its arrays to lists.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote: > +struct action > +{ > + struct list entry; > + WCHAR *action; > +}; > + > typedef struct tagMSIPACKAGE > { > MSIOBJECTHDR hdr; > @@ -403,7 +418,10 @@ typedef struct tagMSIPACKAGE > struct list mimes; > struct list appids; > > - struct tagMSISCRIPT *script; > + struct list script_actions[SCRIPT_MAX]; > + struct list unique_actions; > + BOOL ExecuteSequenceRun; > + UINT InWhatSequence; Why? The package structure is heavy on lists already, some of these should be moved to arrays instead...

2 1

Re: [PATCH] msi/tests: Factor out success checks into table building functions.
by Hans Leidekker Sept. 19, 2017

Sept. 19, 2017

On Tue, 2017-09-19 at 00:23 -0500, Zebediah Figura wrote: > @@ -3457,62 +3474,28 @@ static void test_join(void) > hdb = create_db(); > ok( hdb, "failed to create db\n"); > > - r = create_component_table( hdb ); > - ok( r == ERROR_SUCCESS, "cannot create Component table: %d\n", r ); > - > - r = add_component_entry( hdb, "'zygomatic', 'malar', 'INSTALLDIR', 0, '', ''" ); > - ok( r == ERROR_SUCCESS, "cannot add component: %d\n", r ); Please add a line parameter to the add_* macros so we don't loose that information.

2 1

Re: [resend PATCH] d3d9/tests: Some tests for D3DPRESENTFLAG_LOCKABLE_BACKBUFFER mode
by Henri Verbeet Sept. 19, 2017

Sept. 19, 2017

On 19 September 2017 at 08:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d3d9/tests/device.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 91 insertions(+) > I think this would apply to d3d8 as well. > + hr = IDirect3DDevice9_GetBackBuffer(device, 0, 0, D3DBACKBUFFER_TYPE_MONO, &surface); > + ok(SUCCEEDED(hr), "Failed to get backbuffer, hr %#x.\n", hr); > + > + hr = IDirect3DSurface9_LockRect(surface, &lockrect, NULL, D3DLOCK_DISCARD); > +todo_wine > + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr); ... > + hr = IDirect3DDevice9_Reset(device, &present_parameters); > + ok(SUCCEEDED(hr), "Failed to reset device, hr %#x.\n", hr); It's interesting that that works, actually. Since the LockRect() succeeds on Wine, the surface is mapped when the reset happen. That seems like the kind of thing that shouldn't be allowed.

2 1

Re: [PATCH] msvcrt: Add _vfprintf_l.
by Piotr Caban Sept. 18, 2017

Sept. 18, 2017

Hi, On 09/18/17 03:10, Gijs Vermeulen wrote: > +/********************************************************************* > + * _vfprintf_l (MSVCRT.@) > + */ > +int CDECL MSVCRT__vfprintf_l(MSVCRT_FILE* file, const char *format, > + MSVCRT__locale_t locale, __ms_va_list valist) Please fix the indentation here. Except of that the patch looks good for me. Thanks, Piotr

2 1

Re: [PATCH] propsys: Add PropVariantToBoolean and tests
by Nikolay Sivov Sept. 18, 2017

Sept. 18, 2017

On 18.09.2017 21:20, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/propsys/propsys.spec | 2 +- > dlls/propsys/propvar.c | 58 +++++++++++++++++ > dlls/propsys/tests/propsys.c | 147 +++++++++++++++++++++++++++++++++++++++++++ > include/propvarutil.h | 1 + > 4 files changed, 207 insertions(+), 1 deletion(-) > > diff --git a/dlls/propsys/propsys.spec b/dlls/propsys/propsys.spec > index 582308f5a0..86a5e696eb 100644 > --- a/dlls/propsys/propsys.spec > +++ b/dlls/propsys/propsys.spec > @@ -107,7 +107,7 @@ > @ stub PropVariantGetUInt32Elem > @ stub PropVariantGetUInt64Elem > @ stub PropVariantToBSTR > -@ stub PropVariantToBoolean > +@ stdcall PropVariantToBoolean(ptr ptr) > @ stub PropVariantToBooleanVector > @ stub PropVariantToBooleanVectorAlloc > @ stub PropVariantToBooleanWithDefault > diff --git a/dlls/propsys/propvar.c b/dlls/propsys/propvar.c > index ca3b421ddd..25cebfee02 100644 > --- a/dlls/propsys/propvar.c > +++ b/dlls/propsys/propvar.c > @@ -227,6 +227,64 @@ HRESULT WINAPI PropVariantToUInt64(REFPROPVARIANT propvarIn, ULONGLONG *ret) > return hr; > } > > +HRESULT WINAPI PropVariantToBoolean(REFPROPVARIANT propvarIn, BOOL *ret) > +{ > + LONGLONG res; > + HRESULT hr = S_OK; Initialization does not do anything. > + static const WCHAR wstr_true[] = {'t','r','u','e',0}; > + static const WCHAR wstr_false[] = {'f','a','l','s','e',0}; > + static const CHAR str_true[] = {'t','r','u','e',0}; > + static const CHAR str_false[] = {'f','a','l','s','e',0}; For single byte case you can use string literals right where you need them. > + > + TRACE("%p,%p\n", propvarIn, ret); > + > + *ret = 0; Is it correct to crash on NULL argument? > + > + switch(propvarIn->vt) > + { > + case VT_BOOL: > + *ret = propvarIn->u.boolVal == VARIANT_TRUE; > + return S_OK; > + > + case VT_LPWSTR: > + case VT_BSTR: > + if(!propvarIn->u.pwszVal) > + return DISP_E_TYPEMISMATCH; > + > + if(lstrcmpiW(propvarIn->u.pwszVal, wstr_true) == 0) > + { > + *ret = 1; > + return S_OK; > + } > + if(lstrcmpiW(propvarIn->u.pwszVal, wstr_false) == 0) > + { > + *ret = 0; > + return S_OK; > + } > + break; > + TRUE/FALSE are more common in this context than 0/1 I think. Same with tests. > + case VT_LPSTR: > + if(!propvarIn->u.pszVal) > + return DISP_E_TYPEMISMATCH; > + > + if(lstrcmpiA(propvarIn->u.pszVal, str_true) == 0) > + { > + *ret = 1; > + return S_OK; > + } > + if(lstrcmpiA(propvarIn->u.pszVal, str_false) == 0) > + { > + *ret = 0; > + return S_OK; > + } > + break; > + } > + > + hr = PROPVAR_ConvertNumber(propvarIn, 64, FALSE, &res); > + *ret = !!res; > + return hr; > +} > + > HRESULT WINAPI PropVariantToStringAlloc(REFPROPVARIANT propvarIn, WCHAR **ret) > { > WCHAR *res = NULL; > diff --git a/dlls/propsys/tests/propsys.c b/dlls/propsys/tests/propsys.c > index 776ccd0bfd..5f77f60e5f 100644 > --- a/dlls/propsys/tests/propsys.c > +++ b/dlls/propsys/tests/propsys.c > @@ -883,6 +883,152 @@ static void test_intconversions(void) > ok(llval == -7, "got wrong value %s\n", wine_dbgstr_longlong(llval)); > } > > +static void test_PropVariantToBoolean(void) > +{ > + PROPVARIANT propvar; > + BOOL val; > + HRESULT hr; > + static WCHAR str_0[] = {'0',0}; > + static WCHAR str_1[] = {'1',0}; > + static WCHAR str_7[] = {'7',0}; > + static WCHAR str_true[] = {'t','r','u','e',0}; > + static WCHAR str_true_case[] = {'t','R','U','e',0}; > + static WCHAR str_false[] = {'f','a','l','s','e',0}; > + static WCHAR str_true_space[] = {'t','r','u','e',' ',0}; > + static WCHAR str_yes[] = {'y','e','s',0}; > + static CHAR str_true_ansi[] = {'t','R','U','e',0}; > + static CHAR str_false_ansi[] = {'f','A','L','S','e',0}; > + static CHAR str_1_ansi[] = {'1',0}; > + > + /* test booleans */ > + > + propvar.vt = VT_BOOL; > + propvar.u.boolVal = 0; > + hr = PropVariantToBoolean(&propvar, &val); > + ok(hr == S_OK, "hr=%x\n", hr); > + ok(val == 0, "got wrong value %d\n", val); Better name for 0 in this context is VARIANT_FALSE. > + > + propvar.vt = VT_LPWSTR; > + propvar.u.pwszVal = 0; NULL.

2 2

Re: [PATCH] d3d9/tests: Some tests for D3DPRESENTFLAG_LOCKABLE_BACKBUFFER mode
by Marvin Sept. 18, 2017

Sept. 18, 2017

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=33161 Your paranoid android. === build (build) === Make failed

2 1

Re: [v1 3/3] kernel32/tests: pipe: Added test for pipe security attributes
by Jacek Caban Sept. 18, 2017

Sept. 18, 2017

Hi Jonathan, As I mentioned in the other mail, it would be interesting to add some more tests. As for those tests, I have a few comments bellow. On 12.09.2017 10:01, Jonathan Doron wrote: > Signed-off-by: Jonathan Doron <jond(a)wizery.com> > --- > dlls/kernel32/tests/pipe.c | 241 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 241 insertions(+) > > diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c > index 0a6edd6..9b90ce9 100644 > --- a/dlls/kernel32/tests/pipe.c > +++ b/dlls/kernel32/tests/pipe.c > @@ -28,6 +28,7 @@ > #include "winternl.h" > #include "winioctl.h" > #include "wine/test.h" > +#include "sddl.h" > > #define PIPENAME "\\\\.\\PiPe\\tests_pipe.c" > > @@ -3018,6 +3019,245 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode) > CloseHandle(client); > } > > +static PSECURITY_DESCRIPTOR GetObjectSecDesc(HANDLE Object) > +{ > + ULONG RequiredLength; > + PSECURITY_DESCRIPTOR SecDesc = NULL; > + > + RequiredLength = 0; > + NtQuerySecurityObject(Object, > + GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION, > + NULL, 0, &RequiredLength); // Expected c0000023 > + if (!RequiredLength) > + goto cleanup; > + > + SecDesc = LocalAlloc(LMEM_FIXED, RequiredLength); Please use HeapAlloc(). > + if (!SecDesc) > + goto cleanup; > + memset(SecDesc, 0, RequiredLength); > + > + if (NtQuerySecurityObject(Object, > + GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION, > + SecDesc, RequiredLength, &RequiredLength)) { > + LocalFree(SecDesc); > + SecDesc = NULL; > + goto cleanup; > + } > + > +cleanup: > + return SecDesc; > +} Silent failures are not really good for tests. Please test NtQuerySecurityObject result with ok(). Same for other similar cases. > +static ULONG WINAPI ServerPipeThread(PVOID Param) > +{ > + ULONG RetVal; > + > + RetVal = ConnectNamedPipe((HANDLE)Param, NULL) ? ERROR_SUCCESS : GetLastError(); > + if ((RetVal == ERROR_PIPE_CONNECTED) || (RetVal == ERROR_SUCCESS)) > + RetVal = ERROR_SUCCESS; > + > + ExitThread(RetVal); > +} This is not really needed. You may just connect to file after it's created, skipping new thread and ConnectNamedPipe() call. Also there are styling issues. Please don't use C++ comments (//) in Wine and try to match style of existing code (eg. function names, variable names). Thanks, Jacek

1 0

Re: [v1 2/3] server: Named pipe client needs to use the server SD
by Jacek Caban Sept. 18, 2017

Sept. 18, 2017

Hi Jonathan, On 12.09.2017 10:01, Jonathan Doron wrote: > +static struct security_descriptor *pipe_client_get_sd( struct object *obj ) > +{ > + struct pipe_client *client = (struct pipe_client *) obj; > + if (client->server) > + return default_get_sd((struct object *)client->server); > + return default_get_sd(obj); > +} > + > +static int pipe_client_set_sd( struct object *obj, const struct security_descriptor *sd, > + unsigned int set_info ) > +{ > + struct pipe_client *client = (struct pipe_client *) obj; > + if (client->server) > + return default_set_sd((struct object *)client->server, sd, set_info); > + return default_set_sd(obj, sd, set_info); > +} This looks suspicious to me, it will require more tests. Your tests from the other patch don't really show that we should use the same security descriptor. For example, we could just copy server security descriptor when creating pipe client (and have separated copies later) and it would pass tests as well. It would be interesting add a test that modifies server's security descriptor after client is created and see what happens to client. As for this approach, there are some problems with corner cases like what happens to client's security descriptor when server is destroyed. Thanks, Jacek

1 0

Re: [PATCH] strmiids: Add evr interface GUIDS
by Alexandre Julliard Sept. 18, 2017

Sept. 18, 2017

Fabian Maurer <dark.shadow4(a)web.de> writes: > @@ -18,12 +18,7 @@ > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > */ > > -import "unknwn.idl"; > -import "propidl.idl"; > import "mfidl.idl"; > -import "mftransform.idl"; > -import "mediaobj.idl"; > -import "strmif.idl"; These are included by the MS header, so we need to have them too. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: [PATCH v2 1/5] d3d11.idl: Add missing D3D11_DEVICE_CONTEXT_TYPE enums and D3D11_FEATURE_DATA_* declaration
by Henri Verbeet Sept. 18, 2017

Sept. 18, 2017

On 17 September 2017 at 07:39, Ihsan Akmal <ihsanapps(a)gmail.com> wrote: > +typedef enum D3D11_CONSERVATIVE_RASTERIZATION_TIER { > + D3D11_CONSERVATIVE_RASTERIZATION_NOT_SUPPORTED = 0, > + D3D11_CONSERVATIVE_RASTERIZATION_TIER_1 = 1, > + D3D11_CONSERVATIVE_RASTERIZATION_TIER_2 = 2, > + D3D11_CONSERVATIVE_RASTERIZATION_TIER_3 = 3, > +} D3D11_CONSERVATIVE_RASTERIZATION_TIER; > + > + > +typedef struct D3D11_FEATURE_DATA_D3D11_OPTIONS2 > +{ > + BOOL PSSpecifiedStencilRefSupported; > + BOOL TypedUAVLoadAdditionalFormats; > + BOOL ROVsSupported; > + D3D11_CONSERVATIVE_RASTERIZATION_TIER ConservativeRasterizationTier; > + D3D11_TILED_RESOURCES_TIER TiledResourcesTier; > + BOOL MapOnDefaultTextures; > + BOOL StandardSwizzle; > + BOOL UnifiedMemoryArchitecture; > +} D3D11_FEATURE_DATA_D3D11_OPTIONS2; > + > + The formatting is inconsistent here. > +typedef enum D3D11_SHADER_CACHE_SUPPORT_FLAGS > +{ > + D3D11_SHADER_CACHE_SUPPORT_NONE = 0x0, > + D3D11_SHADER_CACHE_SUPPORT_AUTOMATIC_INPROC_CACHE = 0x1, > + D3D11_SHADER_CACHE_SUPPORT_AUTOMATIC_DISK_CACHE = 0x2, > +} D3D11_SHADER_CACHE_SUPPORT_FLAGS; > + > +typedef struct D3D11_FEATURE_DATA_SHADER_CACHE > +{ > + UINT SupportFlags; > +} D3D11_FEATURE_DATA_SHADER_CACHE; Should "SupportFlags" be of type D3D11_SHADER_CACHE_SUPPORT_FLAGS?

2 3

Re: [PATCH v2 3/5] include: Add dxgi1_3.idl
by Henri Verbeet Sept. 17, 2017

Sept. 17, 2017

On 17 September 2017 at 07:39, Ihsan Akmal <ihsanapps(a)gmail.com> wrote: > +typedef struct DXGI_MATRIX_3X2_F > +{ > + FLOAT _11; > + FLOAT _12; > + FLOAT _21; > + FLOAT _22; > + FLOAT _31; > + FLOAT _32; > +} DXGI_MATRIX_3X2_F; I'd prefer these as "float", although it doesn't really matter. > +typedef enum DXGI_FRAME_PRESENTATION_MODE > +{ > + DXGI_FRAME_PRESENTATION_MODE_COMPOSED»····· = 0, > + DXGI_FRAME_PRESENTATION_MODE_OVERLAY»······ = 1, > + DXGI_FRAME_PRESENTATION_MODE_NONE»· = 2, > + DXGI_FRAME_PRESENTATION_MODE_COMPOSITION_FAILURE»··= 3 > +} DXGI_FRAME_PRESENTATION_MODE; You have some stray tabs there. We generally prefer adding a comma after the last enum element as well, it makes adding elements a little easier. That's pretty minor of course. Applies to DXGI_OVERLAY_SUPPORT_FLAG as well. > +interface IDXGIDevice3 : IDXGIDevice2 > +{ > + void Trim(); > +}; Superfluous semicolon. Applies to all the other interfaces as well. > \ No newline at end of file Alexandre fixes these when committing patches, but please end source files with a newline.

1 0

Re: [PATCH v2 2/5] dxgi1_2.idl: Add missing structs and interfaces
by Henri Verbeet Sept. 17, 2017

Sept. 17, 2017

On 17 September 2017 at 07:39, Ihsan Akmal <ihsanapps(a)gmail.com> wrote: > +interface IDXGIOutputDuplication : IDXGIObject > +{ ... > +}; The semicolon is superfluous. Applies to the IDXGIDisplayControl and IDXGIOutput1 interfaces as well.

1 0

Re: [PATCH] includes: CHOOSECOLOR.lCustData is an LPARAM.
by Nikolay Sivov Sept. 17, 2017

Sept. 17, 2017

On 17.09.2017 11:48, Stefan Dösinger wrote: > This matches the PSDK and MinGW headers. > > Signed-off-by: Stefan Dösinger <stefan(a)codeweavers.com> > --- > include/commdlg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/commdlg.h b/include/commdlg.h > index df56c143c0..eeefce2bfe 100644 > --- a/include/commdlg.h > +++ b/include/commdlg.h > @@ -183,7 +183,7 @@ typedef struct { > DWORD rgbResult; > LPDWORD lpCustColors; > DWORD Flags; > - DWORD lCustData; > + LPARAM lCustData; > LPCCHOOKPROC lpfnHook; > LPCSTR lpTemplateName; > } CHOOSECOLORA; > @@ -196,7 +196,7 @@ typedef struct { > DWORD rgbResult; > LPDWORD lpCustColors; > DWORD Flags; > - DWORD lCustData; > + LPARAM lCustData; > LPCCHOOKPROC lpfnHook; > LPCWSTR lpTemplateName; > } CHOOSECOLORW; > Another difference, not near as important, is that SDK and mingw are using COLORREFs for color fields.

1 0

Re: [PATCH 1/5] d3d11.idl: Add missing D3D11_DEVICE_CONTEXT_TYPE enums and D3D11_FEATURE_DATA_* declaration
by Alex Henrie Sept. 17, 2017

Sept. 17, 2017

Hello Ihsan, and thank you for your interest in contributing to Wine! Please note that when submitting patches, you must use your real name in the From and Signed-off-by lines. You can find this and other helpful instructions at <https://wiki.winehq.org/Submitting_Patches>. -Alex

1 0

Re: [PATCH v2 1/3] mshtml: Add a null parameter check in navigate_new_window().
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

On 15.09.2017 07:34, Zebediah Figura wrote: > v2: check in caller instead of do_query_service(). > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > dlls/mshtml/navigate.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c > index 423d650a8c..06425c3f2b 100644 > --- a/dlls/mshtml/navigate.c > +++ b/dlls/mshtml/navigate.c > @@ -2246,6 +2246,12 @@ HRESULT navigate_new_window(HTMLOuterWindow *window, IUri *uri, const WCHAR *nam > nsChannelBSC *bsc; > HRESULT hres; > > + if (!window->doc_obj->client) > + { > + FIXME("no attached client site; cannot query INewWindowManager\n"); > + return E_NOTIMPL; > + } This doesn't look right. Look at HlinkTarget_Navigate. We explicitly choose this code path because client is NULL. This seems to be a regression from commit 63205bf01d6 and the right solution seems to be simply ignoring the fact that we don't have INewWindowManager and continue. Thanks, Jacek

2 1

Re: [PATCH v4 2/2] ieframe: Add stub implementation of InternetExplorerManager.
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

On 15.09.2017 07:43, Zebediah Figura wrote: > +HRESULT WINAPI InternetExplorerManager_Create(IClassFactory *iface, IUnknown *pOuter, REFIID riid, void **ppv) > +{ > + InternetExplorerManager *ret; > + HRESULT hr; > + > + 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; > + ret->ref = 1; > + > + hr = IInternetExplorerManager_QueryInterface(&ret->IInternetExplorerManager_iface, riid, ppv); > + IInternetExplorerManager_Release(&ret->IInternetExplorerManager_iface); > + > + return hres; That doesn't compile. Jacek

2 1

Re: [PATCH] dsquery: Register CommonQuery coclass.
by Alexandre Julliard Sept. 15, 2017

Sept. 15, 2017

Zebediah Figura <zfigura(a)codeweavers.com> writes: > + > +[ > + threading(apartment), > + uuid(83bc5ec0-6f2a-11d0-a1c4-00aa00c16e65) > +] > +coclass CommonQuery { interface ICommonQuery; } This should most likely go in the public header. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] d3d9: Ask for texture GetDC() capability GET_DC based on texture format instead of initial swapchain buffer format
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 15 September 2017 at 07:14, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Initially application sets buffer format to UNKNOWN that is resolved later > in wined3d to something compatible with GetDC() functionality. > A small test would be great.

1 0

Re: [PATCH 2/2] d2d1: Implement GetBounds() for path geometries
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 14 September 2017 at 14:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + /* GetBounds() with bezier segments. */ > + hr = ID2D1Factory_CreatePathGeometry(factory, &geometry); > + ok(SUCCEEDED(hr), "Failed to create path geometry, hr %#x.\n", hr); > + hr = ID2D1PathGeometry_Open(geometry, &sink); > + ok(SUCCEEDED(hr), "Failed to open geometry sink, hr %#x.\n", hr); > + fill_geometry_sink_bezier(sink); > + hr = ID2D1GeometrySink_Close(sink); > + ok(SUCCEEDED(hr), "Failed to close geometry sink, hr %#x.\n", hr); > + ID2D1GeometrySink_Release(sink); > + > + set_rect(&rect, 0.0f, 0.0f, 0.0f, 0.0f); > + hr = ID2D1PathGeometry_GetBounds(geometry, NULL, &rect); > + ok(SUCCEEDED(hr), "Failed to get geometry bounds, hr %#x.\n", hr); > + match = compare_rect(&rect, 5.0f, 20.0f, 75.0f, 752.0f, 0); > + ok(match, "Got unexpected rectangle {%.8e, %.8e, %.8e, %.8e}.\n", > + rect.left, rect.top, rect.right, rect.bottom); Note that the way fill_geometry_sink_bezier() is constructed, the bounds tend to coincide with the control points, probably even under a carefully chosen rotation. The figure from test_bezier_intersect() under various rotations may be more interesting.

1 0

Re: [PATCH 1/2] d2d1: Use better bezier segment bounds approximation when updating figure bounds
by Henri Verbeet Sept. 15, 2017

Sept. 15, 2017

On 14 September 2017 at 14:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d2d1/geometry.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > This is fine, but note that d2d_geometry_sink_AddQuadraticBeziers() also exists.

1 0

Re: [PATCH v2 1/2] secur32: Fix race between schan_(Encrypt|Decrypt)Message
by Jacek Caban Sept. 15, 2017

Sept. 15, 2017

Hi Anton, On 14.09.2017 18:30, Anton Romanov wrote: > ctx->req_ctx_attr = fContextReq; > > - transport.ctx = ctx; > - init_schan_buffers(&transport.in, pInput, schan_init_sec_ctx_get_next_input_buffer); > - transport.in.limit = expected_size; > - init_schan_buffers(&transport.out, pOutput, schan_init_sec_ctx_get_next_output_buffer); > - schan_imp_set_session_transport(ctx->session, &transport); > + ctx->transport.ctx = ctx; One last thing, please move this line closer to schan_imp_create_session, where it belongs. transport.ctx should be set once, during context creation, and not touched later. Other than that, the patch looks good. Thanks, Jacek

2 2

Re: [2/10] wsdapi: Add helper functions for populating XML strings.
by Huw Davies Sept. 15, 2017

Sept. 15, 2017

On Thu, Sep 14, 2017 at 10:54:17PM +0100, Owen Rudge wrote: > Signed-off-by: Owen Rudge <orudge(a)codeweavers.com> > --- > dlls/wsdapi/Makefile.in | 3 +- > dlls/wsdapi/soap.c | 86 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 dlls/wsdapi/soap.c > > Unfortunately adding unused helper functions isn't the right way to split up the patches. You'll need to find a way to break up the functionality while incrementally adding these things. > diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c > new file mode 100644 > index 0000000..9662203 > --- /dev/null > +++ b/dlls/wsdapi/soap.c > +static inline void free_xml_string(WS_XML_STRING *value) > +{ > + HANDLE heap; > + > + if (value == NULL) > + return; > + > + heap = GetProcessHeap(); > + > + if (value->bytes != NULL) > + HeapFree(heap, 0, value->bytes); > + > + HeapFree(heap, 0, value); > +} > GetProcessHeap() is cheap, you don't need to bother caching it. You may want to consider using the standardized heap_alloc() / heap_free() helpers. Huw.

1 0

Re: [PATCH 1/3] mshtml: Add a null parameter check in do_query_service().
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

Hi Zebediah, On 14.09.2017 20:42, Zebediah Figura wrote: > diff --git a/dlls/mshtml/main.c b/dlls/mshtml/main.c > index af716d1680..43270b9cc8 100644 > --- a/dlls/mshtml/main.c > +++ b/dlls/mshtml/main.c > @@ -190,6 +190,9 @@ HRESULT do_query_service(IUnknown *unk, REFGUID guid_service, REFIID riid, void > IServiceProvider *sp; > HRESULT hres; > > + if (!unk) > + return E_FAIL; I think it would be cleaner to add NULL check to caller. Thanks, Jacek

2 1

Re: [PATCH v2 2/2] secur32: Protect SSLRead/Write with cs on OSX
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

Hi Anton, On 14.09.2017 18:30, Anton Romanov wrote: > @@ -742,6 +743,9 @@ BOOL schan_imp_create_session(schan_imp_session *session, schan_credentials *cre > if (!s) > return FALSE; > > + InitializeCriticalSection(&s->cs); > + DEBUG_SET_CRITSEC_NAME(&s->cs, "schannel_session"); AFAICS this won't compile, this macro is not defined. If you don't have Mac to test, that's fine. I will fix it and test for you. Jacek

1 0

Re: [PATCH 1/2] secur32: Split push/pull schannel transports
by Jacek Caban Sept. 14, 2017

Sept. 14, 2017

On 13.09.2017 07:33, Anton Romanov wrote: > This makes schan_EncryptMessage/schan_DecryptMessage thread safe between > each other. > MSDN states that it's supposed to be thread safe. > Fixes Magic The Gathering: Online crash: > https://bugs.winehq.org/show_bug.cgi?id=43453 > > Signed-off-by: Anton Romanov <theli.ua(a)gmail.com> > --- > dlls/secur32/schannel.c | 33 +++++++++++++++++++-------------- > dlls/secur32/schannel_gnutls.c | 12 +++++++----- > dlls/secur32/schannel_macosx.c | 21 ++++++++++++--------- > dlls/secur32/secur32_priv.h | 5 +++-- > 4 files changed, 41 insertions(+), 30 deletions(-) > > diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c > index 82374efd55..1f938e1a37 100644 > --- a/dlls/secur32/schannel.c > +++ b/dlls/secur32/schannel.c > @@ -59,6 +59,8 @@ struct schan_context > schan_imp_session session; > ULONG req_ctx_attr; > const CERT_CONTEXT *cert; > + struct schan_transport *push_transport; > + struct schan_transport *pull_transport; Do you really need separated transports? schan_transport already has separated in and out buffers. It seems to me that as long as you use only in buffers in reader and out buffers in writer (and it's already the case), there is no need for separated transport instances. Also, why do you use pointers here? Couldn't you just store the struct inside schan_context? > diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c > index bc3bbaf67d..97b2c2d767 100644 > --- a/dlls/secur32/schannel_gnutls.c > +++ b/dlls/secur32/schannel_gnutls.c > @@ -73,7 +73,7 @@ MAKE_FUNCPTR(gnutls_record_send); > MAKE_FUNCPTR(gnutls_server_name_set); > MAKE_FUNCPTR(gnutls_transport_get_ptr); > MAKE_FUNCPTR(gnutls_transport_set_errno); > -MAKE_FUNCPTR(gnutls_transport_set_ptr); > +MAKE_FUNCPTR(gnutls_transport_set_ptr2); It's related to the previous comment, it seems to me that gnutls_transport_set_ptr should be enough. Thanks, Jacek

2 1

Re: [PATCH v2] riched20: Add more warnings for unhandled control words.
by Huw Davies Sept. 14, 2017

Sept. 14, 2017

On Thu, Sep 14, 2017 at 01:03:11AM -0500, Zebediah Figura wrote: > v2: use FIXME instead of WARN; use debugstr_a (thanks Huw) Unfortunately, these generate a lot of noise (especially in the tests). We probably want to deal with things like \rtf and \uc before this goes in. Huw.

1 0

Re: [PATCH] riched20: Add more warnings for unhandled control words.
by Huw Davies Sept. 14, 2017

Sept. 14, 2017

On Tue, Sep 12, 2017 at 09:43:10AM -0500, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/riched20/editor.c | 6 ++++++ > dlls/riched20/reader.c | 15 +++++++++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index a7b1bc748f..f6badb1a36 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -533,6 +533,8 @@ void ME_RTFCharAttrHook(RTF_Info *info) > if (info->rtfParam != rtfNoParam) > fmt.yHeight = info->rtfParam*10; > break; > + default: > + WARN("unknown token \"%s\"\n", info->rtfTextBuf); These should be FIXME()s not WARN()s. It would also be better to use debugstr_a() on the string. > @@ -2450,7 +2454,10 @@ static void > Destination (RTF_Info *info) > { > if (!RTFGetDestinationCallback(info, info->rtfMinor)) > + { > + WARN("unknown token \"%s\"\n", info->rtfTextBuf); > RTFSkipGroup (info); > + } > } Now would be a good opportunity to get rid of the tabs from this function. Huw.

2 1

Re: [PATCH 5/7] msi: Perform the given action, or INSTALL, from MsiInstallProduct().
by Hans Leidekker Sept. 13, 2017

Sept. 13, 2017

On Tue, 2017-09-12 at 20:51 -0500, Zebediah Figura wrote: > The removed code to re-run all actions after InstallValidate is old > and not supported by tests. This makes the tests fail here: err:msi:ITERATE_Actions Execution halted, action L"Run C" returned 1603 Makefile:326: recipe for target 'install.ok' failed make: *** [install.ok] Error 1

2 2

Re: [PATCH 3/7] msi: More fully implement the ExecuteAction action.
by Hans Leidekker Sept. 13, 2017

Sept. 13, 2017

On Tue, 2017-09-12 at 20:51 -0500, Zebediah Figura wrote: > +static UINT iterate_properties(MSIRECORD *record, void *param) > +{ > + static const WCHAR prop_template[] = > + {'P','r','o','p','e','r','t','y','(','S',')',':',' ','[','1',']',' ','=',' ','[','2',']',0}; > + MSIRECORD *uirow; > + > + uirow = MSI_CloneRecord(record); > + if (!uirow) return ERROR_OUTOFMEMORY; > + MSI_RecordSetStringW(uirow, 0, prop_template); > + MSI_ProcessMessage((MSIPACKAGE *)param, INSTALLMESSAGE_INFO|MB_ICONHAND, uirow); You don't need this cast. > static UINT ACTION_ExecuteAction(MSIPACKAGE *package) > { > + static const WCHAR prop_query[] = > + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ','`','_','P','r','o','p','e','r','t','y','`',0}; > + WCHAR *productname; > + WCHAR *action = msi_dup_property(package->db, szEXECUTEACTION); > + MSIQUERY *view; > + MSIRECORD *uirow; > UINT rc; > > - package->script->InWhatSequence |= SEQUENCE_EXEC; > - rc = ACTION_ProcessExecSequence(package,FALSE); > + if (!action) action = msi_strdupW(szINSTALL, strlenW(szINSTALL)); action is leaked. > + /* Send COMMONDATA and INFO messages. */ > + /* FIXME: when should these messages be sent? [see also MsiOpenPackage()] */ > + uirow = MSI_CreateRecord(3); > + if (!uirow) return ERROR_OUTOFMEMORY; > + MSI_RecordSetStringW(uirow, 0, NULL); > + MSI_RecordSetInteger(uirow, 1, 0); > + MSI_RecordSetInteger(uirow, 2, package->num_langids ? package->langids[0] : 0); > + MSI_RecordSetInteger(uirow, 3, msi_get_string_table_codepage(package->db->strings)); > + MSI_ProcessMessageVerbatim(package, INSTALLMESSAGE_COMMONDATA, uirow); > + /* FIXME: send INSTALLMESSAGE_PROGRESS */ > + MSI_ProcessMessageVerbatim(package, INSTALLMESSAGE_COMMONDATA, uirow); > + MSI_ProcessMessage(package, INSTALLMESSAGE_COMMONDATA, uirow); > + > + productname = msi_dup_property(package->db, INSTALLPROPERTY_PRODUCTNAMEW); productname is leaked on error paths.

2 1

Re: [PATCH 4/4] mshtml: Expose IHTMLElement6 to scripts.
by Marvin Sept. 13, 2017

Sept. 13, 2017

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=33084 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 5/5] wined3d: Don't take the upload code path if the destination surface is onscreen.
by Henri Verbeet Sept. 13, 2017

Sept. 13, 2017

On 12 September 2017 at 21:22, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> > --- > dlls/wined3d/surface.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Why would we want to avoid that?

2 2

Re: [v4 1/4] strmiids: Add evr interface GUIDS
by Alexandre Julliard Sept. 12, 2017

Sept. 12, 2017

Fabian Maurer <dark.shadow4(a)web.de> writes: > evr GUIDs should be defined in strmiids.lib This seems to be adding many GUIDs that are not supposed to be there. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] comctl32/tests: Dump sequence correctly
by Nikolay Sivov Sept. 12, 2017

Sept. 12, 2017

On 09/12/2017 07:17 PM, Fabian Maurer wrote: > The "expected" pointer already got moved in the function, > make sure to use the full list of expected messages. Yes, that makes sense, thanks. Please make it look closer to how it's done in user32, see attached diff.

2 1

Re: [PATCH 1/2] comctl32/taskdialog: Initial support for callback procedure
by Fabian Maurer Sept. 12, 2017

Sept. 12, 2017

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: [v4 2/4] evr: Add tests
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33066 Your paranoid android. === build (build) === Make failed

2 1

Re: [1/6] wsdapi: Add wsdapi_internal.h for shared internal function and macro definitions. [try 3]
by Owen Rudge Sept. 12, 2017

Sept. 12, 2017

Hi Alexandre, Is there anything else that needs improving for these patches to be accepted? Thanks, -- Owen Rudge http://www.owenrudge.net/

2 1

Re: [PATCH 7/7] mshtml: Added IHTMLDocument7::getElementsByClassName implementation.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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

1 0

Re: [PATCH 6/7] mshtml: Added IHTMLDocument7::head property implementation.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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=33061 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 5/7] mshtml: Expose IHTMLDocument7 interface to scripts.
by Marvin Sept. 12, 2017

Sept. 12, 2017

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

1 0