April 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 1/13] msidb: Add stub tool for manipulating MSI databases.
by Hans Leidekker April 26, 2017

April 26, 2017

On Sun, 2017-04-09 at 13:12 -0600, Erich E. Hoover wrote: > + /* process and validate all the command line flags */ > + for (i = 1; n && i < argc; i += n) > + n = process_argument( &state, i, argc, argv ); > + if (!valid_state( &state )) > + return 1; > + > + /* perform the requested operations */ > + if (!open_database( &state )) > + { > + ERR( "Failed to open database '%s'.\n", wine_dbgstr_w(state.database_file) ); For programs it should be WINE_ERR, etc.

2 1

Re: po: Update Simplified Chinese translation.
by Alexandre Julliard April 26, 2017

April 26, 2017

Jactry Zeng <jzeng(a)codeweavers.com> writes: > diff --git a/po/zh_CN.po b/po/zh_CN.po > index 45103bf3e4..f2addad1a5 100644 > --- a/po/zh_CN.po > +++ b/po/zh_CN.po > @@ -27,7 +27,7 @@ msgstr "Š»œçŒ¡ç®£çºŒ¯æ'®£'" > > #: appwiz.rc:62 > msgid "&Install..." > -msgstr "®£(&I)" > +msgstr "®£(&I)..." Is it really necessary to change all the ellipsis characters? They were changed the other way around not so long ago. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [v4 PATCH] odbccp32: Handle ODBC_CONFIG_DRIVER request in SQLConfigDriver/W
by Huw Davies April 26, 2017

April 26, 2017

On Wed, Apr 26, 2017 at 12:46:49AM +0000, Alistair Leslie-Hughes wrote: > diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c > index f28d464..475b74e 100644 > --- a/dlls/odbccp32/odbccp32.c > +++ b/dlls/odbccp32/odbccp32.c > @@ -65,6 +65,7 @@ static const WCHAR odbc_error_invalid_param_string[] = {'I','n','v','a','l','i', > static const WCHAR odbc_error_invalid_dsn[] = {'I','n','v','a','l','i','d',' ','D','S','N',0}; > static const WCHAR odbc_error_load_lib_failed[] = {'L','o','a','d',' ','L','i','b','r','a','r','y',' ','F','a','i','l','e','d',0}; > static const WCHAR odbc_error_request_failed[] = {'R','e','q','u','e','s','t',' ','F','a','i','l','e','d',0}; > +static const WCHAR odbc_error_invalid_keyword[] = {'I','n','v','a','l','i','d',' ','k','e','y','w','o','r','d',' ','v','a','l','u','e',0}; > > /* Push an error onto the error stack, taking care of ranges etc. */ > static void push_error(int code, LPCWSTR msg) > @@ -303,6 +304,64 @@ static HMODULE load_config_driver(const WCHAR *driver) > return hmod; > } > > +static BOOL write_config_value(const WCHAR *driver, const WCHAR *args) > +{ > + long ret; > + HKEY hkey, hkeydriver; > + WCHAR *name = NULL; > + > + if (!args) > + return FALSE; > + > + if ((ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, odbcini, &hkey)) == ERROR_SUCCESS) > + { > + if ((ret = RegOpenKeyW(hkey, driver, &hkeydriver)) == ERROR_SUCCESS) > + { > + WCHAR *divider, *value; > + > + name = heap_alloc( (strlenW(args) + 1) * sizeof(WCHAR)); > + if(!name) > + { > + push_error(ODBC_ERROR_OUT_OF_MEM, odbc_error_out_of_mem); > + goto fail; > + } > + lstrcpyW(name, args); > + > + divider = strchrW(name,'='); > + if(!divider) > + { > + push_error(ODBC_ERROR_INVALID_KEYWORD_VALUE, odbc_error_invalid_keyword); > + goto fail; > + } > + > + value = divider + 1; > + *divider = '\0'; > + > + TRACE("Write pair: %s = %s\n", debugstr_w(name), debugstr_w(value)); > + if(RegSetValueExW(hkeydriver, name, 0, REG_SZ, (BYTE*)value, > + strlenW(value) * sizeof(WCHAR)) != ERROR_SUCCESS) The length should be (strlenW(value) + 1) * sizeof(WCHAR). Sorry, I missed that last time. While you're at it, decide whether you're going to use a space after 'if' or not, and then be consistent. Otherwise, this looks good. Huw.

1 0

Re: [PATCH v2 1/2] mountmgr: Create devices and registry entries for serial ports.
by Alexandre Julliard April 26, 2017

April 26, 2017

Alex Henrie <alexhenrie24(a)gmail.com> writes: > + if (driver == serial_driver) > + { > + dos_name_format = comW; > + nt_name_format = device_serialW; > + reg_value_format = comW; > + } > + else > + { > + /* TODO: support parallel ports */ > + } Please avoid introducing dead code. You can add the if() in the second patch. > + /* remove old symlinks */ > + for (n = 1; n <= MAX_PORTS; n++) > + { > + sprintf( p, "%u", n ); > + if (unlink( dosdevices_path ) != 0 && !errno) > + break; > + } Why would errno ever be 0 here? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: dmscript/tests: Use a void* instead of void** outer IUnknown argument
by Marvin April 25, 2017

April 25, 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=30436 Your paranoid android. === wxppro (32 bit dmscript) === dmscript.c:146: Test failed: DirectMusicScriptTrack create failed: 80004003, expected CLASS_E_NOAGGREGATION === wvistau64 (32 bit dmscript) === 01f4:dmscript: unhandled exception c0000005 at 73E47D60 === w7u (32 bit dmscript) === 0ac8:dmscript: unhandled exception c0000005 at 72157EA8 === w7pro64 (32 bit dmscript) === 0978:dmscript: unhandled exception c0000005 at 73907EA8 === w8 (32 bit dmscript) === 0938:dmscript: unhandled exception c0000005 at 69E4804C === w864 (32 bit dmscript) === 0f8c:dmscript: unhandled exception c0000005 at 7451804C === w1064 (32 bit dmscript) === 02bc:dmscript: unhandled exception c0000005 at 7078945E

2 1

Re: [v2 6/6] d3dx9: Support parameters sharing in effect.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 13:36 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +struct d3dx_shared_data > +{ > + void *data; > + LONG refcount; > + struct d3dx_parameter **parameters; > + unsigned int table_size; > +}; A bit nitpicky but generally for those dynamic arrays we call "refcount" "count" and "table_size" "size". > + new_refcount = InterlockedIncrement(&pool->shared_data[i].refcount); I think you don't need the Interlocked functions for this count given that adding / removing multiple effects to a pool is expected to be racy. > + if (new_refcount >= pool->shared_data[i].table_size) > + { > + if (!pool->shared_data[i].table_size) > + { > + new_size = INITIAL_SHARED_DATA_TABLE_SIZE; > + pool->shared_data[i].parameters = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + sizeof(*pool->shared_data[i].parameters) * INITIAL_SHARED_DATA_TABLE_SIZE); > + } > + else > + { > + new_size = pool->shared_data[i].table_size * 2; > + pool->shared_data[i].parameters = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + pool->shared_data[i].parameters, > + sizeof(*pool->shared_data[i].parameters) * new_size); > + } > + pool->shared_data[i].table_size = new_size; > + } > + > + param->shared_data = &pool->shared_data[i]; > + pool->shared_data[i].parameters[new_refcount - 1] = param; > + > + TRACE("name %s, parameter idx %u, new refcount %u.\n", param->name, i, > + new_refcount); > + return; > +} Unnecessary return. > +static BOOL param_zero_data_func(void *dummy, struct d3dx_parameter *param) > +{ > + param->data = NULL; > + return FALSE; > +} > + > +static void d3dx_pool_release_shared_parameter(struct d3dx_parameter *param) > +{ > + LONG new_refcount; > + > + if (!(param->flags & PARAMETER_FLAG_SHARED) || !param->shared_data) > + return; No need to check for the PARAMETER_FLAG_SHARED flag. Maybe it makes sense to assert that the flag is there if param->shared_data is not NULL. > static void free_effect_pool(struct d3dx_effect_pool *pool) > { > + unsigned int i; > + > + for (i = 0; i < pool->table_size; ++i) > + { > + if (pool->shared_data[i].refcount) > + ERR("Releasing pool with referenced parameters, shared data is not freed.\n"); > + } This can be triggered by an application so it should be a WARN(). BTW, this probably needs a test. On pool destruction, the effects still using it might be "unshared" i.e. data from the pool might be moved back to the effects. Or maybe the effects still using the pool are simply broken and accessing parameter data after the pool is destroyed crashes.

2 6

Re: [PATCH 2/5] ddraw: Ignore unsupported blit flags.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 23:31 GMT+02:00 Henri Verbeet <hverbeet(a)codeweavers.com>: > Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> > --- > dlls/ddraw/surface.c | 24 +++++++++++++++++++++--- > dlls/ddraw/tests/ddraw1.c | 4 ++-- > dlls/ddraw/tests/ddraw2.c | 4 ++-- > dlls/ddraw/tests/ddraw4.c | 4 ++-- > dlls/ddraw/tests/ddraw7.c | 4 ++-- > 5 files changed, 29 insertions(+), 11 deletions(-) > > diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c > index 6c4ed11..90731f0 100644 > --- a/dlls/ddraw/surface.c > +++ b/dlls/ddraw/surface.c > @@ -1570,6 +1570,7 @@ static HRESULT WINAPI DECLSPEC_HOTPATCH ddraw_surface7_Blt(IDirectDrawSurface7 * > struct ddraw_surface *dst_impl = impl_from_IDirectDrawSurface7(iface); > struct ddraw_surface *src_impl = unsafe_impl_from_IDirectDrawSurface7(src_surface); > struct wined3d_blt_fx wined3d_fx; > + DWORD unsupported_flags; > DWORD fill_colour = 0; > HRESULT hr = DD_OK; > DDBLTFX rop_fx; > @@ -1577,9 +1578,26 @@ static HRESULT WINAPI DECLSPEC_HOTPATCH ddraw_surface7_Blt(IDirectDrawSurface7 * > TRACE("iface %p, dst_rect %s, src_surface %p, src_rect %s, flags %#x, fx %p.\n", > iface, wine_dbgstr_rect(dst_rect), src_surface, wine_dbgstr_rect(src_rect), flags, fx); > > - /* Check for validity of the flags here. WineD3D Has the software-opengl selection path and would have > - * to check at 2 places, and sometimes do double checks. This also saves the call to wined3d :-) > - */ > + unsupported_flags = DDBLT_ALPHADEST > + | DDBLT_ALPHADESTCONSTOVERRIDE > + | DDBLT_ALPHADESTNEG > + | DDBLT_ALPHADESTSURFACEOVERRIDE > + | DDBLT_ALPHAEDGEBLEND > + | DDBLT_ALPHASRC > + | DDBLT_ALPHASRCCONSTOVERRIDE > + | DDBLT_ALPHASRCNEG > + | DDBLT_ALPHASRCSURFACEOVERRIDE > + | DDBLT_ZBUFFER > + | DDBLT_ZBUFFERDESTCONSTOVERRIDE > + | DDBLT_ZBUFFERDESTOVERRIDE > + | DDBLT_ZBUFFERSRCCONSTOVERRIDE > + | DDBLT_ZBUFFERSRCOVERRIDE; Hardly going to matter but wouldn't it be better as a static const?

2 1

Re: [v4 PATCH 2/2] wined3d: Update only dirty regions in wined3d_device_update_texture().
by Marvin April 25, 2017

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

1 0

Re: [v2 4/6] d3dx9: Reference effect pool implementation in d3dx9_base_effect structure.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 13:36 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > /*** IUnknown methods ***/ > -static HRESULT WINAPI ID3DXEffectPoolImpl_QueryInterface(ID3DXEffectPool *iface, REFIID riid, void **object) > +static HRESULT WINAPI d3dx_effect_pool_QueryInterface(ID3DXEffectPool *iface, REFIID riid, void **object) > { > TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), object); Please update / cleanup this TRACE too. > -static ULONG WINAPI ID3DXEffectPoolImpl_AddRef(ID3DXEffectPool *iface) > +static ULONG WINAPI d3dx_effect_pool_AddRef(ID3DXEffectPool *iface) > { > - struct ID3DXEffectPoolImpl *This = impl_from_ID3DXEffectPool(iface); > + struct d3dx_effect_pool *pool = impl_from_ID3DXEffectPool(iface); > > - TRACE("(%p)->(): AddRef from %u\n", This, This->ref); > + TRACE("iface %p, pool %p, ref %u.\n", iface, pool, pool->ref); Tracing both iface and pool doesn't seem useful and I don't think I've ever seen it anywhere else. Also for AddRef() we usually go with a TRACE in the form: TRACE("%p increasing refcount to %u.\n", iface, refcount); Similarly for Release(). Have a look at recent implementations of the IUnknown functions like e.g. d3d11_hull_shader_* in dlls/d3d11/shader.c.

1 0

Re: [PATCH 1/4] comdlg32: Release the returned imagelist.
by Huw Davies April 25, 2017

April 25, 2017

On Tue, Apr 25, 2017 at 09:04:56AM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/comdlg32/filedlg.c | 2 ++ > 1 file changed, 2 insertions(+) Actually, while this is correct, could you skip it for now? I'm going to have to make other changes to this area, so it would be better to hold off committing this one. The other patches in the series are still good. Huw.

1 0

Re: [PATCH 2/4] regedit: Simplify parseKeyName()
by Alexandre Julliard April 25, 2017

April 25, 2017

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > - > - for (i = 0; i < ARRAY_SIZE(reg_class_keys); i++) { > - if (CompareStringW(LOCALE_USER_DEFAULT, 0, lpKeyName, len, reg_class_namesW[i], -1) == CSTR_EQUAL && > - len == lstrlenW(reg_class_namesW[i])) { > + num_class_keys = ARRAY_SIZE(reg_class_keys); > + for (i = 0; i < num_class_keys; i++) > + { > + if (!strncmpW(lpKeyName, reg_class_namesW[i], lstrlenW(reg_class_namesW[i]))) > + { You still need to check that the lengths match. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] wined3d: Check pointer before use
by Józef Kucia April 25, 2017

April 25, 2017

On Tue, Apr 25, 2017 at 8:01 AM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > Test was provided by Józef Kucia. > > Fixes https://bugs.winehq.org/show_bug.cgi?id=42873 > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/d3d11/tests/d3d11.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ > dlls/wined3d/context.c | 2 +- > 2 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c > index dfe50c6287..2961cc06cb 100644 > --- a/dlls/d3d11/tests/d3d11.c > +++ b/dlls/d3d11/tests/d3d11.c > @@ -17567,6 +17567,63 @@ done: > release_test_context(&test_context); > } > > +static void test_destroyed_wined3d_context_query_poll(void) FWIW, I'm not sure if we want to use this name for the test. I'm sorry that I haven't mentioned this earlier in the bug report. > --- a/dlls/wined3d/context.c > +++ b/dlls/wined3d/context.c > @@ -3968,7 +3968,7 @@ struct wined3d_context *context_reacquire(const struct wined3d_device *device, > { > struct wined3d_context *current_context; > > - if (context->tid != GetCurrentThreadId()) > + if (!context || context->tid != GetCurrentThreadId()) > return NULL; > > current_context = context_acquire(device, context->current_rt.texture, I think it's fine to do a NULL check in context_reacquire(), and it should be good enough in practice (i.e. this corner case shouldn't matter much). However we may want to handle this correctly in the future. I think it may be good idea to add a similar test for D3D11_QUERY_OCCLUSION (or replace this one). For the occlusion query we want to additionally test the result of the query which is expected to be incorrect (todo_wine).

1 0

Re: [PATCH] programs/winecfg: enable CSMT checkbox on Graphics tab
by Henri Verbeet April 25, 2017

April 25, 2017

On 24 April 2017 at 23:01, Donat Enikeev <donat(a)enikeev.net> wrote: > @@ -181,6 +181,9 @@ BEGIN > EDITTEXT IDC_RES_DPIEDIT,188,105,23,13,ES_NUMBER|WS_TABSTOP > LTEXT "#msgctxt#unit: dots/inch#dpi",IDC_STATIC,215,107,30,8 > LTEXT "This is a sample text using 10 point Tahoma",IDC_RES_FONT_PREVIEW,15,124,230,49 > + > + GROUPBOX "CSMT",IDC_STATIC,8,186,244,32 > + CONTROL "Enable multi-threaded command stream in Direct3D",IDC_ENABLE_CSMT,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,15,202,230,8 > END > > IDD_DLLCFG DIALOG 0, 0, 260, 220 I don't think so, this is not a setting regular users should be modifying.

2 3

`setupapi` patch from staging
by Donat Enikeev April 24, 2017

April 24, 2017

Hi Guys, Number of recent well-known games are based on Starling game engine (Adobe Air+Stage3D for gpu rendering), such as: The Banner Saga 2, Samorost 3, The Inner World, Angry Birds. They are are falling into Software rendering mode on plain Wine and thus barely playable (bug #41635) Thanks to Béla Gyebrószki finding @ bug #41653, Wine makes these games consider d3d9 rendering with the following single patch from wine-staging https://github.com/wine-compholio/wine-staging/blob/master/patches/setupapi… This patch implements support of the case when a full driver path is passed to SetupDiGetClassDevs (trace successfully reaching AddDeviceToSet for a game below [1]). I was unable to find @wine-devel discussion around this patch, I would be grateful if one could kindly remind why it was staged, probably it just requires refactoring around the change or additional tests? I am asking this, because Adobe stopped Linux support for the Air years ago and we could never see such games ported, and therefore plain Wine's support of gpu acceleration for such games looks important. Thanks in advance Best regards, Donnie [1] 0009:trace:setupapi:SetupDiGetClassDevsExW {4d36e968-e325-11ce-bfc1-08002be10318} L"PCI\\VEN_0000&DEV_0000" (nil) 0x0000000a (nil) (null) (nil) 0009:warn:setupapi:SetupDiGetClassDevsExW unsupported flags 0000000a 0009:trace:setupapi:SetupDiCreateDeviceInfoListExW {4d36e968-e325-11ce-bfc1-08002be10318} (nil) (null) (nil) 0009:trace:setupapi:SETUPDI_EnumerateDevices 0x464ac90, {4d36e968-e325-11ce-bfc1-08002be10318}, L"PCI\\VEN_0000&DEV_0000", 0000000a 0009:trace:setupapi:SETUPDI_EnumerateMatchingDeviceInstances L"PCI" L"VEN_0000&DEV_0000" 0009:trace:setupapi:SETUPDI_AddDeviceToSet 0x464ac90, {4d36e968-e325-11ce-bfc1-08002be10318}, 0, L"PCI\\VEN_0000&DEV_0000\\13&12345&0", 0

3 6

Re: [PATCH] wined3d: Add Nvidia TitanX (Pascal)
by Henri Verbeet April 24, 2017

April 24, 2017

On 23 April 2017 at 21:12, Christopher Berner <christopherberner(a)gmail.com> wrote: > + {HW_VENDOR_NVIDIA, CARD_NVIDIA_TITANX_PASCAL, "NVIDIA TITAN X (Pascal)", DRIVER_NVIDIA_GEFORCE8, 12288}, Is that really what it's called by the Windows driver?

2 1

Re: [PATCH] shell32/ebrowser: Added IOleWindow stub for IExplorerBrowser
by Huw Davies April 24, 2017

April 24, 2017

On Mon, Apr 24, 2017 at 08:50:09AM +0300, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/shell32/ebrowser.c | 56 +++++++++++++++++++++++++++++++++++++++++++ > dlls/shell32/tests/ebrowser.c | 2 +- > 2 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c > index bbccf88814..77c727a5ec 100644 > --- a/dlls/shell32/ebrowser.c > +++ b/dlls/shell32/ebrowser.c > @@ -56,6 +56,7 @@ typedef struct _ExplorerBrowserImpl { > IShellBrowser IShellBrowser_iface; > ICommDlgBrowser3 ICommDlgBrowser3_iface; > IObjectWithSite IObjectWithSite_iface; > + IOleWindow IOleWindow_iface; > INameSpaceTreeControlEvents INameSpaceTreeControlEvents_iface; > IInputObject IInputObject_iface; > LONG ref; IShellBrowser inherits IOleWindow, so you don't need this. Huw.

2 1

Re: [PATCH] comctl32/taskdialog: Implement buttons.
by Nikolay Sivov April 24, 2017

April 24, 2017

On 23.03.2017 21:22, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/comctl32/taskdialog.c | 224 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 161 insertions(+), 63 deletions(-) > > diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c > index 6b8bcfcae6..037b85e082 100644 > --- a/dlls/comctl32/taskdialog.c > +++ b/dlls/comctl32/taskdialog.c > @@ -41,10 +41,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(taskdialog); > #define ALIGN_LENGTH(_Len, _Align) _Len = ALIGNED_LENGTH(_Len, _Align) > #define ALIGN_POINTER(_Ptr, _Align) _Ptr = ALIGNED_POINTER(_Ptr, _Align) > > -static const UINT DIALOG_MIN_WIDTH = 180; > +static const UINT DIALOG_MIN_WIDTH = 240; > static const UINT DIALOG_SPACING = 5; > -static const UINT DIALOG_BUTTON_WIDTH = 50; > -static const UINT DIALOG_BUTTON_HEIGHT = 14; > +static const UINT DIALOG_BUTTON_WIDTH_MIN = 45; > +static const UINT DIALOG_BUTTON_HEIGHT = 13; This was taken from MSGBOX template, what's a point in making it different? > +static const UINT DIALOG_SPACING_BUTTONS_LEFT = 30; /* minimum distance from the left dialog border */ > +static const UINT DIALOG_SPACING_BUTTON_H = 1; /* Distance between buttons */ > +static const UINT DIALOG_SPACING_BUTTON_W = 4; /* Distance between buttons */ > > static const UINT ID_MAIN_INSTRUCTION = 0xf000; > > @@ -67,6 +70,15 @@ struct taskdialog_template_desc > HFONT font; > }; > > +typedef struct > +{ > + int id; > + const WCHAR *text; > + UINT width; > + UINT x; > + UINT y; > +} button_info; > + > static void pixels_to_dialogunits(const struct taskdialog_template_desc *desc, LONG *width, LONG *height) > { > if (width) > @@ -83,6 +95,44 @@ static void dialogunits_to_pixels(const struct taskdialog_template_desc *desc, L > *height = MulDiv(*height, desc->y_baseunit, 8); > } > > +/* used to calculate size for the controls */ > +static RECT text_get_rect(const struct taskdialog_template_desc *desc, const WCHAR *text) > +{ > + const WCHAR *textW = NULL; > + unsigned int length; > + HFONT oldfont; > + HDC hdc; > + static const WCHAR nulW; > + RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* padding left and right of the control */ > + > + if (IS_INTRESOURCE(text)) > + { > + if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)text, (WCHAR *)&textW, 0))) > + { > + WARN("Failed to load text\n"); > + textW = &nulW; > + length = 0; > + } > + } > + else > + { > + textW = text; > + length = strlenW(textW); > + } > + > + hdc = GetDC(0); > + oldfont = SelectObject(hdc, desc->font); > + > + dialogunits_to_pixels(desc, &rect.right, NULL); > + DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS | DT_CALCRECT | DT_WORDBREAK); > + pixels_to_dialogunits(desc, &rect.right, &rect.bottom); > + > + SelectObject(hdc, oldfont); > + ReleaseDC(0, hdc); > + > + return rect; > +} > + Did you test that buttons are supposed to support multiple lines of text? > static void template_write_data(char **ptr, const void *src, unsigned int size) > { > memcpy(*ptr, src, size); > @@ -90,7 +140,7 @@ static void template_write_data(char **ptr, const void *src, unsigned int size) > } > > static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc, WORD id, const WCHAR *class, > - HINSTANCE hInstance, const WCHAR *text, short x, short y, short cx, short cy) > + DWORD style, HINSTANCE hInstance, const WCHAR *text, short x, short y, short cx, short cy) > { > struct taskdialog_control *control = Alloc(sizeof(*control)); > unsigned int size, class_size, text_size; > @@ -117,7 +167,7 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc > control->template = template = Alloc(size); > control->template_size = size; > > - template->style = WS_VISIBLE; > + template->style = WS_VISIBLE | WS_CHILD | style; You don't use styles in this patch, this is unnecessary change. > template->dwExtendedStyle = 0; > template->x = x; > template->y = y; > @@ -136,78 +186,126 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc > > static unsigned int taskdialog_add_main_instruction(struct taskdialog_template_desc *desc) > { > - RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* padding left and right of the control */ > - const WCHAR *textW = NULL; > - unsigned int size, length; > - HFONT oldfont; > - HDC hdc; > + unsigned int size; > + RECT rect; > > if (!desc->taskconfig->pszMainInstruction) > return 0; > > - if (IS_INTRESOURCE(desc->taskconfig->pszMainInstruction)) > - { > - if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)desc->taskconfig->pszMainInstruction, > - (WCHAR *)&textW, 0))) > - { > - WARN("Failed to load main instruction text\n"); > - return 0; > - } > - } > - else > - { > - textW = desc->taskconfig->pszMainInstruction; > - length = strlenW(textW); > - } > - > - hdc = GetDC(0); > - oldfont = SelectObject(hdc, desc->font); > - > - dialogunits_to_pixels(desc, &rect.right, NULL); > - DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS | DT_CALCRECT | DT_WORDBREAK); > - pixels_to_dialogunits(desc, &rect.right, &rect.bottom); > - > - SelectObject(hdc, oldfont); > - ReleaseDC(0, hdc); > + rect = text_get_rect(desc, desc->taskconfig->pszMainInstruction); > > - size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, desc->taskconfig->hInstance, > + size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, 0, desc->taskconfig->hInstance, > desc->taskconfig->pszMainInstruction, DIALOG_SPACING, desc->dialog_height, > rect.right, rect.bottom); > - desc->dialog_height += rect.bottom; > + desc->dialog_height += rect.bottom + DIALOG_SPACING; > return size; > } > > -static unsigned int taskdialog_add_common_buttons(struct taskdialog_template_desc *desc) > +static button_info make_button(struct taskdialog_template_desc *desc, int id, const WCHAR *text) > +{ > + RECT rect; > + button_info button; > + > + button.id = id; > + button.text = text; > + rect = text_get_rect(desc, text); > + button.width = rect.right + 10; > + if (button.width < DIALOG_BUTTON_WIDTH_MIN) > + button.width = DIALOG_BUTTON_WIDTH_MIN; > + > + return button; > +} This does not handle "text" correctly. > + > +static unsigned int taskdialog_add_buttons(struct taskdialog_template_desc *desc) > { > - short button_x = desc->dialog_width - DIALOG_BUTTON_WIDTH - DIALOG_SPACING; > DWORD flags = desc->taskconfig->dwCommonButtons; > + const TASKDIALOGCONFIG *taskconfig = desc->taskconfig; > + UINT alignment = DIALOG_SPACING_BUTTONS_LEFT; /* minimum distance from the left dialog border */ > + UINT location_x; > + BOOL first_row = TRUE; > + button_info *buttons; > + int count = 0; > + int i; > unsigned int size = 0; > > -#define TASKDIALOG_ADD_COMMON_BUTTON(id) \ > - do { \ > - size += taskdialog_add_control(desc, ID##id, WC_BUTTONW, COMCTL32_hModule, MAKEINTRESOURCEW(IDS_BUTTON_##id), \ > - button_x, desc->dialog_height + DIALOG_SPACING, DIALOG_BUTTON_WIDTH, DIALOG_BUTTON_HEIGHT); \ > - button_x -= DIALOG_BUTTON_WIDTH + DIALOG_SPACING; \ > - } while(0) > - if (flags & TDCBF_CLOSE_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(CLOSE); > - if (flags & TDCBF_CANCEL_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(CANCEL); > - if (flags & TDCBF_RETRY_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(RETRY); > - if (flags & TDCBF_NO_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(NO); > - if (flags & TDCBF_YES_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(YES); > + /* Allocate enough memory for the custom and the default buttons */ > + if (taskconfig->cButtons && taskconfig->pButtons) > + buttons = HeapAlloc(GetProcessHeap(), 0, (taskconfig->cButtons + 6) * sizeof(button_info)); > + else > + { > + buttons = HeapAlloc(GetProcessHeap(), 0, 6 * sizeof(button_info)); > + } > + > + /* Custom buttons */ > + if (taskconfig->cButtons && taskconfig->pButtons) > + { > + for (i = 0; i < taskconfig->cButtons; i++) > + { > + buttons[count++] = make_button(desc, taskconfig->pButtons[i].nButtonID, taskconfig->pButtons[i].pszButtonText); > + } > + } > + > + /* Default buttons */ > if (flags & TDCBF_OK_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > - /* Always add OK button */ > - if (list_empty(&desc->controls)) > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > -#undef TASKDIALOG_ADD_COMMON_BUTTON > - > - /* make room for common buttons row */ > - desc->dialog_height += DIALOG_BUTTON_HEIGHT + 2 * DIALOG_SPACING; > + buttons[count++] = make_button(desc, IDOK, MAKEINTRESOURCEW(IDS_BUTTON_OK)); > + if (flags & TDCBF_YES_BUTTON) > + buttons[count++] = make_button(desc, IDYES, MAKEINTRESOURCEW(IDS_BUTTON_YES)); > + if (flags & TDCBF_NO_BUTTON) > + buttons[count++] = make_button(desc, IDNO, MAKEINTRESOURCEW(IDS_BUTTON_NO)); > + if (flags & TDCBF_RETRY_BUTTON) > + buttons[count++] = make_button(desc, IDRETRY, MAKEINTRESOURCEW(IDS_BUTTON_RETRY)); > + if (flags & TDCBF_CANCEL_BUTTON) > + buttons[count++] = make_button(desc, IDCANCEL, MAKEINTRESOURCEW(IDS_BUTTON_CANCEL)); > + if (flags & TDCBF_CLOSE_BUTTON) > + buttons[count++] = make_button(desc, IDCLOSE, MAKEINTRESOURCEW(IDS_BUTTON_CLOSE)); > + > + /* There must be at least one button */ > + if (count == 0) > + buttons[count++] = make_button(desc, IDOK, MAKEINTRESOURCEW(IDS_BUTTON_OK)); > + > + /* Position buttons */ > + location_x = alignment; > + for (i = 0; i < count; i++) > + { > + /* When beginning new row, align the first */ > + if (location_x + buttons[i].width + DIALOG_SPACING_BUTTON_W > desc->dialog_width) > + { > + if (first_row) > + { > + int diff = desc->dialog_width - location_x; > + > + first_row = FALSE; > + for (int j = 0; j < i; j++) /* Align first row to the right */ > + buttons[j].x += diff; > + alignment = buttons[0].x; /* left-align all coming rows to the first row */ > + } > + location_x = alignment; > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + DIALOG_SPACING_BUTTON_H; > + } > + > + buttons[i].x = location_x; > + buttons[i].y = desc->dialog_height; > + > + location_x += buttons[i].width + DIALOG_SPACING_BUTTON_W; > + } > + if (first_row) /* Always align first row to the right */ > + { > + int diff = desc->dialog_width - (buttons[count - 1].x + buttons[count - 1].width + DIALOG_SPACING_BUTTON_W); > + for (int i = 0; i < count; i++) > + buttons[i].x += diff; > + } I don't like this. Why do you need temporary array, and iterating twice seems excessive. > + > + /* Now that we got them all positioned, create all buttons */ > + for (i = 0; i < count; i++) > + { > + size += taskdialog_add_control(desc, buttons[i].id, WC_BUTTONW, BS_PUSHBUTTON, COMCTL32_hModule, > + buttons[i].text, buttons[i].x, buttons[i].y, buttons[i].width, DIALOG_BUTTON_HEIGHT); > + } This won't work for custom buttons. > + > + /* Add height for last row and spacing */ > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + DIALOG_SPACING; > + > + HeapFree(GetProcessHeap(), 0, buttons); > return size; > } > > @@ -295,7 +393,7 @@ static DLGTEMPLATE *create_taskdialog_template(const TASKDIALOGCONFIG *taskconfi > desc.dialog_width = min(desc.dialog_width, screen_width); > > size += taskdialog_add_main_instruction(&desc); > - size += taskdialog_add_common_buttons(&desc); > + size += taskdialog_add_buttons(&desc); I'd rather have it separated if possible. I'll do some testing myself, before commenting further. > > template = Alloc(size); > if (!template) >

2 8

Build server and Staging patch submission
by Michael Müller April 24, 2017

April 24, 2017

Hi all, I would like to announce some changes regarding the build server and the patch submission for Wine Staging. As already mentioned in the last email about the packaging changes, one of the main ideas was to implement an easy to use web interface, so that wine developers can create their own builds. As you can most probably imagine, such a system is quite complex (updating VMs, building packages, signing packages, updating repos, synchronizing files, ...) but I think we have now reached a state in which we can put the system into a public beta. There are some tasks left (like adding support for FreeBSD), but the main system works and was already used to create the 2.5, 2.6 and 2.0.1 WineHQ packages. I have created a wiki article which describes how to get access to the builder and basically everything else you need to know. You can find it at https://wiki.winehq.org/Build_Server. There is another change we would like to announce. Since Wine Staging was merged into WineHQ, there was no longer an official way to suggest patches. This lead to a non-transparent system of private emails, requests in bug reports etc. This is not something you would expect from an open source project, so we took the opportunity and also included a patch tracker into the website. Submissions and comments are now public for everyone and you can also subscribe to changes. For more information about the system and how to submit patches, take look at https://wiki.winehq.org/Wine-Staging_Patches. We have tested the patch tracker in a closed beta phase, so you can also find some older submissions there. Both systems are available at https://dev.wine-staging.com/. They use the same login system, so you don't need to registers two accounts. Alternatively, you can just use Github for the login, then just have to verify your email address. Everything else should be described in the corresponding Wiki articles. So, try it out and drop me an email if you find any issues! Regards, Michael

1 0

Re: a-m-w-c-processthreads-l1-1-0: Forward OpenThreadToken() to advapi32.
by Sebastian Lackner April 23, 2017

April 23, 2017

On 24.04.2017 00:30, Olivier F. R. Dierick wrote: > Copy and paste of the stdcall from the 1.1.1 spec file. > > Application Psyprax needs this [1]. > > [1] https://forum.winehq.org/viewtopic.php?f=9&t=27888 > > Signed-off-by: Olivier F. R. Dierick <o.dierick(a)piezo-forte.be> > --- > .../api-ms-win-core-processthreads-l1-1-0.spec > | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > It would be better to update tools/make_specfiles, so that advapi32 stuff is synchronized automatically for this dll. Best regards, Sebastian

1 0

Re: ntdll: Mark LDR data as initialized.
by Marvin April 23, 2017

April 23, 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=30362 Your paranoid android. === 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: [v3 PATCH 1/2] wined3d: Add dirty region tracking.
by Henri Verbeet April 23, 2017

April 23, 2017

On 20 April 2017 at 16:01, Masanori Kakura <kakurasan(a)gmail.com> wrote: > v2 - Use array > - Regions are stored in struct wined3d_texture > - Reuse old region if possible > v3 - Update Wine d3d8 test > - Get rid of "Ignoring dirty_region" FIXME in wined3d_texture_add_dirty_region() > > Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com> > --- > dlls/d3d8/tests/visual.c | 10 ++--- > dlls/d3d9/tests/visual.c | 10 ++--- > dlls/wined3d/cs.c | 51 +++++++++++++++++++++- > dlls/wined3d/device.c | 6 +++ > dlls/wined3d/texture.c | 97 ++++++++++++++++++++++++++++++++++++++++-- > dlls/wined3d/wined3d_private.h | 9 +++- > 6 files changed, 167 insertions(+), 16 deletions(-) > Do you have any thoughts on the interactions with the resource location management?

2 2

Re: [PATCH 5/5] ddraw/tests: Introduce a helper function to create windows.
by Marvin April 23, 2017

April 23, 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=30354 Your paranoid android. === w864 (32 bit ddraw7) === ddraw7.c:3724: Test failed: Lit quad without normals has color 0x00ff0000, expected 0x00000000. ddraw7.c:3728: Test failed: Lit quad with normals has color 0x00ff0000, expected 0x00000000. ddraw7.c:3752: Test failed: Lit quad with light has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with singular world matrix has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with transformation matrix has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with non-affine matrix has color 0x00ff0000.

1 0

Slow emulation of StretchBlt
by andrea April 22, 2017

April 22, 2017

Hi, I am looking at the performance of AppleWin (a 32 bit app) in Wine and have come across this function StretchBlt which seems to be much slower in Wine that in Windows 10. The call looks like this StretchBlt( hFrameDC, xdest, ydest, wdest, hdest, g_hDeviceDC, xSrc, ySrc, FRAMEBUFFER_BORDERLESS_W, FRAMEBUFFER_BORDERLESS_H, SRCCOPY); With typical values xdest 0 int ydest 0 int wdest 1120 int hdest 768 int xSrc 22 int ySrc 17 int #define FRAMEBUFFER_BORDERLESS_W 560 #define FRAMEBUFFER_BORDERLESS_H 384 In Wine they take 20x the time in Windows 10 (running inside a VirtualBox). The consequence is CPU utilisation 20x higher in Wine than Windows. I am running wine 2.5 in a 64 bit Fedora 25 machine. Does this ring any bell? Andrea

5 6

Debian Download Page
by Stefan Leichter April 22, 2017

April 22, 2017

Hi, when following the instruction at https://wiki.winehq.org/Debian the command "sudo apt-get update" gives me the error: -------------------------------------------------------------------- W: GPG error: https://dl.winehq.org jessie InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 818A435C5FCBF54A -------------------------------------------------------------------- Does anyone else see the same error? Minor item: Any reason that "wine-stable" is not listed on the download page? Regards Stefan

2 2

Linux desktop integration: Fill StartupWMClass in .desktop menu entries
by Eike Hein April 22, 2017

April 22, 2017

Hi, I maintain the taskbar in Plasma 5 and would like to poke folks about a desktop integration pain point we (and our friends at other DEs) face with apps run via wine. Here's the relevant ticket: https://bugs.winehq.org/show_bug.cgi?id=32699 Let me give some broader context on this. Modern DEs have a swath of UX going on that relies on being able to identify the application owning a particular window in the windowing system. "Identify the application" in practice means "map the window to a .desktop file installed in the menu file system". We want to do this because: a) Many a taskbar/dock-like UI tries to form a smooth life cycle across "launcher button", "startup notification" and "window button(s)". For each of those we need to work out the app they are associated with to be able to match them up. If it fails, ugly things like co-existing task buttons or missing startup throbbers and similar happen. b) UI these days tends to prefer to load assets such as app icons via the values defined in .desktop files instead of using the window icon, to ensure consistent results (apps may load icon assets differently from the shell, leading to asset quality issues, inconsistent theming, the icon changing between the lifecycle states mentioned in 'a', etc.) Plus the Wayland windowing system we're all migrating towards currently doesn'ㅅ have window icons. c) DEs do things like track app starts/quits for things like "Most used apps" launcher lists. To match a window to a .desktop file we rely on window and process metadata. On X11, there's the WM_CLASS window property: https://tronche.com/gui/x/icccm/sec-4.html#WM_CLASS Ideally, the second part of a X11 window's WM_CLASS matches the name of a .desktop file installed on the system. Failing that, it matches the value of StartupWMClass in that .desktop file (this is the key part I'll get back to soon). Here's some more documentation by our friends at Gnome on this standard scheme: https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased (Look for WM_CLASS / StartupWMClass.) In practice, we pull many other tricks to cope with broken meta data. We try to match things by names, we grab process command lines from /proc and check them against Exec keys in .desktop files in complicated ways, ... it's a multi-page heuristic, and other DEs (Unity via bamf, Gnome Shell via iirc ShellWindowTracker) have equivalent code. Something Gnome does that we don't do (because I think it's brittle) is to store the name of the .desktop file an app was launched with as a private window hint on the next window that comes up (which then doesn't work when the app was launched from CLI instead of shell UI). None of these things currently allow us to handle wine very well. As of now (perhaps thanks to the patch mentioned in the linked ticket), wine sets WM_CLASS to the executable name of the app, e.g. "Notepad.exe". But the .desktop files it generates don't match this by name, and they don't contain a StartupWMClass key. The process command line is the Windows-like path to the .exe, which we also can't easily match up to the complicated Exec values in wine's .desktop files. (Aside: On Wayland, there's a more modern version of this: Windows have an appid property, which should be set to a .desktop file name. I don't know what the status of Wayland support in wine is -- currently the wine clients we encounter run over Xwayland, so we're back to WM_CLASS. We hope wine will set this property correctly when the time comes, or we're happy to work with you guys to find a way to support wine.) Long story short ... could this be picked up? The goal must be to have either .desktop file names or their StartupWMClass values match whatever is set as WM_CLASS. Currently, the easiest fix would be to set StartupWMClass to the executable name set as WM_CLASS. I assume the .desktop file generator has access to this, as the .lnk file referred to by Exec should presumably contain the name of the executable to be launched. Cheers, Eike

2 2

Re: [PATCH 3/4] d3d9/tests: Accept alpha channel differences in shadow_test.
by Marvin April 21, 2017

April 21, 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=30337 Your paranoid android. === w8 (32 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

2 1

Re: [PATCH 1/4] ddraw/tests: Don't use DDBLT_COLORFILL in test_flip (v2).
by Marvin April 21, 2017

April 21, 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=30335 Your paranoid android. === w8 (32 bit ddraw1) === ddraw1.c:6945: Test failed: Got unexpected color 0x00000000. ddraw1.c:6949: Test failed: Got unexpected color 0x00000000. ddraw1.c:7012: Test failed: Got unexpected color 0x00000000. ddraw1.c:7014: Test failed: Got unexpected color 0x00000000.

2 1

Re: [Tools v2] winetest/dissect: Check the build id and get its details in one Git call.
by Alexandre Julliard April 21, 2017

April 21, 2017

Francois Gouget <fgouget(a)codeweavers.com> writes: > get_build_info() only returns an empty subject if the commit does not > exist. It may still exist but not be a proper commit SHA-1. I'm not sure it's a good idea to relax that test. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 7/8] d3dx9: Support parameters sharing in effect.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/d3dx9_private.h | 11 ++- > dlls/d3dx9_36/effect.c | 197 +++++++++++++++++++++++++++++++++++++++++- > dlls/d3dx9_36/tests/effect.c | 17 ++-- > 3 files changed, 211 insertions(+), 14 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index da4564d..d6d4280 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -215,15 +215,22 @@ struct d3dx_parameter > struct d3dx_parameter *annotations; > struct d3dx_parameter *members; > > - struct d3dx_parameter *referenced_param; > struct d3dx_param_eval *param_eval; > > struct d3dx_parameter *top_level_param; > + union > + { > + struct d3dx_parameter *referenced_param; > + struct d3dx_parameter *shared_param; > + }; > + LONG shared_refcount; > }; I think you mentioned this before but it shouldn't be necessary to store the whole parameter structure in the pool. I guess it could be improved after the fact but it shouldn't be too complicated to do it right away. Probably it could just be shared_param_list[] from patch 8/8 and shared_refcount (which at that point won't need to be in struct d3dx_parameter anymore), in addition to data. You could access the other parameter fields through shared_param_list[0]. > +static void d3dx_pool_sync_shared_parameter(struct ID3DXEffectPoolImpl *pool, struct d3dx_parameter *param) > +{ > + unsigned int i, free_entry_index; > + LONG new_refcount; > + > + if (!(param->flags & PARAMETER_FLAG_SHARED) || !pool > + || is_param_type_sampler(param->type)) > + return; > + > + free_entry_index = pool->parameter_count; > + for (i = 0; i < pool->parameter_count; ++i) > + { > + if (!pool->parameters[i]->shared_refcount) > + free_entry_index = i; > + else if (is_same_parameter(param, pool->parameters[i])) > + break; > + } > + if (i == pool->parameter_count) > + { > + i = free_entry_index; > + if (i >= pool->table_size) > + { > + struct d3dx_parameter ** new_alloc; No whitespace after "**", please. > + else > + { > + new_size = pool->table_size * 2; > + new_alloc = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, pool->parameters, > + sizeof(*pool->parameters) * new_size); > + if (!new_alloc) > + { > + ERR("Out of memory.\n"); > + return; > + } > + } > + pool->parameters = new_alloc; > + pool->table_size = new_size; > + } > + pool->parameters[i] = HeapAlloc(GetProcessHeap(), > + HEAP_ZERO_MEMORY, sizeof(*pool->parameters[i])); > + copy_parameter_structure(param, pool->parameters[i], NULL, FALSE); > + param_set_data_pointer(pool->parameters[i], (unsigned char *)param->data, FALSE, FALSE); > + if (i == pool->parameter_count) > + pool->parameter_count++; > + } I don't think this works. HeapReAlloc might move the memory block and that would break the preexisting effects using the pool (and also the previous parameters of the new effect). HEAP_REALLOC_IN_PLACE_ONLY would avoid that but it also makes it easier for the reallocation to fail. I see two ways to fix this: - refresh all the parameters of the effects that are in the pool when you have to reallocate (shared_param_list[] might help here) - allocate the memory in chunks, keeping an array (or list) of chunks which you allocate (but never resize) as needed

2 1

Re: [Tools v2] winetest: Get rid of $gitdir.
by Alexandre Julliard April 21, 2017

April 21, 2017

Francois Gouget <fgouget(a)codeweavers.com> writes: > WineHQ appears to use /home/winehq/opt/source/git/wine.git as the Wine > checkout directory. After this patch the scripts will use > $workdir/wine.git so this should most likely symlink to the above > location. I'm not sure that's an improvement. > However any non-official site will actually need write access to it from > the scripts to ensure that wine.git is up to date before processing > reports. Otherwise the reports risk being rejected due to the not yet > known commit id. This could still work with a placement in tools/winehq: > one would just have to symlink that to a writable location but it does > imply placing it in $workdir may make more sense. I don't think it makes sense to put it in the website directory, it's not meant to be accessed from the web. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 3/8] d3dx9: Reference top level parameter instead of dirty flag.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > -static BOOL param_set_dirty_flag_ptr(void *dirty_flag_ptr, struct d3dx_parameter *param) > +static BOOL param_set_top_level_param(void *top_level_param, struct d3dx_parameter *param) > { > - param->dirty_flag_ptr = (DWORD *)dirty_flag_ptr; > + param->top_level_param = (struct d3dx_parameter *)top_level_param; > return FALSE; > } The cast is unnecessary (yeah, it was already unnecessary, I missed that earlier). It might make sense to store both? Avoiding the extra indirection in is_param_dirty() might help a tiny bit. It's fine to tackle this later on.

2 2

Re: [PATCH 8/8] d3dx9: Use local dirty flags for shared parameters.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: As I somewhat hinted at in the previous patch, this one should probably be merged with 7/8. It did help to keep them separated for the first review pass though :) > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index d6d4280..f9cbf7c 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -222,15 +222,14 @@ struct d3dx_parameter > { > struct d3dx_parameter *referenced_param; > struct d3dx_parameter *shared_param; > + struct d3dx_parameter **shared_param_list; This name is a bit confusing given that it is an array of (pointers to) matching parameters from different effects and shared_param is a pointer to the shared pool parameter. It becomes less of an issue if the new field moves to the separate shared parameter structure I mentioned. Maybe just call it "parameters"? > @@ -3193,7 +3196,16 @@ static void d3dx_pool_sync_shared_parameter(struct ID3DXEffectPoolImpl *pool, st > param_set_data_pointer(param, (unsigned char *)pool->parameters[i]->data, FALSE, TRUE); > } > new_refcount = InterlockedIncrement(&pool->parameters[i]->shared_refcount); > + if (new_refcount == 1) > + pool->parameters[i]->shared_param_list = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + sizeof(*pool->parameters[i]->shared_param_list)); > + else > + pool->parameters[i]->shared_param_list = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + pool->parameters[i]->shared_param_list, > + sizeof(*pool->parameters[i]->shared_param_list) * new_refcount); It's probably better to make an initial allocation for > 1 elements and then expand by doubling the size, as usual with this kind of stuff.

1 0

Re: [PATCH 6/8] d3dx9: Factor out free_parameter_data() function.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void free_parameter(struct d3dx_parameter *param, BOOL element, BOOL child) > +{ > + unsigned int i; > + > + TRACE("Free parameter %p, name %s, type %s, child %s\n", param, param->name, > + debug_d3dxparameter_type(param->type), child ? "yes" : "no"); Again, given that it's a chance for some cleanup, please add the period at the end of the message. Also I'd change the "child" part, either just using %#x or child ? "child" : "" (or something else, the point being that the current "child yes" vs "child no" looks pretty awkward). Maybe you can also trace the "element" parameter. > + if (param->param_eval) > + { > + d3dx_free_param_eval(param->param_eval); > + param->param_eval = NULL; Is setting the field to NULL necessary? Same for the others below.

1 0

Re: [PATCH 5/8] d3dx9: Reference effect pool implementation in d3dx9_base_effect structure.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +struct ID3DXEffectPoolImpl > +{ > + ID3DXEffectPool ID3DXEffectPool_iface; > + LONG ref; > +}; Please rename it to "d3dx_effect_pool" or similar, while you're at it. Same for the interface methods (d3dx_effect_pool_QueryInterface(), ...) We want to get rid of the MS-like names for our implementation and this seems like a good chance to clean this one up. > - TRACE("base %p, data %p, data_size %lu, effect %p\n", base, data, data_size, effect); > + TRACE("base %p, data %p, data_size %lu, effect %p, pool %p\n", base, data, data_size, effect, pool); Since you're touching it, please add a period at the end of the message.

1 0

Re: [PATCH 2/8] d3dx9/tests: Add more tests for IsParameterUsed() function.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + todo_wine > + ok(effect3->lpVtbl->IsParameterUsed(effect3, "vs_arr2", "tech0"), > + "Unexpected IsParameterUsed result FALSE.\n"); > + todo_wine > + ok(!effect3->lpVtbl->IsParameterUsed(effect3, "vs_arr2[0]", "tech0"), > + "Unexpected IsParameterUsed result FALSE.\n"); Typo. I'd just leave out the TRUE / FALSE from these messages, one has to look up the code anyway if there is a test failure.

1 0

Re: [2/2] [v2] secur32: Add support for SECPKG_ATTR_KEY_INFO.
by Jacek Caban April 21, 2017

April 21, 2017

Hi Akihiro, The patch looks mostly good now, thanks. I have a few minor comments. On 20.04.2017 13:58, Akihiro Sagawa wrote: > +struct schan_alg_id_name { > + ALG_ID alg_id; > + const char* name; > + const WCHAR *nameW; > +}; Using an array here like const WCHAR nameW[8]; would make later initialization nicer by not requiring multiple string variables. > + > +static int comp_alg_id(const void *a, const void *b) > +{ > + const struct schan_alg_id_name *lhs = a; > + const struct schan_alg_id_name *rhs = b; > + return (int)lhs->alg_id - (int)rhs->alg_id; > +} > + > +static void* get_alg_name(ALG_ID id, BOOL wide) > +{ > + static const WCHAR ecdsaW[] = {'E','C','D','S','A',0}; > + static const WCHAR rsaW[] = {'R','S','A',0}; > + static const WCHAR desW[] = {'D','E','S',0}; > + static const WCHAR rc2W[] = {'R','C','2',0}; > + static const WCHAR triple_desW[] = {'3','D','E','S',0}; > + static const WCHAR aesW[] = {'A','E','S',0}; > + static const WCHAR rc4W[] = {'R','C','4',0}; > + static const struct schan_alg_id_name alg_name_map[] = { > + { CALG_ECDSA, "ECDSA", ecdsaW }, > + { CALG_RSA_SIGN, "RSA", rsaW }, > + { CALG_DES, "DES", desW }, > + { CALG_RC2, "RC2", rc2W }, > + { CALG_3DES, "3DES", triple_desW }, > + { CALG_AES_128, "AES", aesW }, > + { CALG_AES_192, "AES", aesW }, > + { CALG_AES_256, "AES", aesW }, > + { CALG_RC4, "RC4", rc4W }, > + }; > + const struct schan_alg_id_name *res; > + struct schan_alg_id_name key; > + > + key.alg_id = id; > + res = bsearch(&key, alg_name_map, > + sizeof(alg_name_map)/sizeof(alg_name_map[0]), sizeof(alg_name_map[0]), > + comp_alg_id); You could use id directly as a key. You don't need it to be of schan_alg_id_name* type, the key may be just ALG_ID. Thanks, Jacek

1 0

Re: [PATCH] tests: Don't evaluate the todo condition when not running on Wine.
by Sebastian Lackner April 21, 2017

April 21, 2017

On 21.04.2017 15:01, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > This cropped up because I wanted to write a todo condition involving > IsProcessDPIAware(), since this doesn't exist in XP, its address is > loaded dynamically. I could have written the condition as > pIsProcessDPIAware && pIsProcessDPIAware(), but since I know the > function is always present in Wine, that's unnecessary. Perhaps > more to the point, I didn't expect the current behaviour. > > include/wine/test.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/wine/test.h b/include/wine/test.h > index af602c0fac..fa3f90c932 100644 > --- a/include/wine/test.h > +++ b/include/wine/test.h > @@ -123,7 +123,7 @@ extern void __winetest_cdecl winetest_trace( const char *msg, ... ) WINETEST_PRI > winetest_loop_todo(); \ > winetest_end_todo()) > #define todo_wine todo_if(!strcmp(winetest_platform, "wine")) > -#define todo_wine_if(is_todo) todo_if((is_todo) && !strcmp(winetest_platform, "wine")) > +#define todo_wine_if(is_todo) todo_if(!strcmp(winetest_platform, "wine") && (is_todo)) > > > #ifdef NONAMELESSUNION > I don't mind if its changed, but please note that it was intentional to check "is_todo" first. The idea was that side effects (like assigning variables, changing last error, ...) are applied on both Wine and Windows. Just think about something like: FirstCall(); todo_wine_if(SecondCall()) ok(GetLastError() == ..., "\n"); With your patch applied, it will test two entirely different things on Windows (error from FirstCall) and Wine (error from SecondCall), which would also be confusing. Best regards, Sebastian

3 3

Re: [PATCH] msvcrt: Handle synchronous flag for x64 C++ exceptions
by Piotr Caban April 21, 2017

April 21, 2017

Hi, On 04/20/17 16:42, Daniel Lehman wrote: > + if (descr->magic >= CXX_FRAME_MAGIC_VC8 && > + (descr->flags & FUNC_DESCR_SYNCHRONOUS) && > + (rec->ExceptionCode != CXX_EXCEPTION && rec->ExceptionCode != STATUS_UNWIND_CONSOLIDATE)) > + return ExceptionContinueSearch; /* handle only c++ exceptions */ I think the condition for STATUS_UNWIND_CONSOLIDATE should also check number of parameters and ExceptionInformation[0]==(ULONG_PTR)call_catch_block like it's done during unwinding. Also the code breaks unwinding for longjump when FUNC_DESCR_SYNCHRONOUS flag is set. Thanks, Piotr

1 0

Re: [PATCH 08/12] d3d10core/tests: Allow passing any interface to get_refcount().
by Henri Verbeet April 21, 2017

April 21, 2017

On 21 April 2017 at 10:59, Józef Kucia <jkucia(a)codeweavers.com> wrote: > -static ULONG get_refcount(IUnknown *iface) > +#define get_refcount(iface) get_refcount_((IUnknown *)iface) > +static ULONG get_refcount_(IUnknown *iface) You don't really need that macro.

1 0

Linux desktop integration: Fill StartupWMClass in .desktop menu entries
by Eike Hein April 21, 2017

April 21, 2017

Hi, I maintain the taskbar in Plasma 5 and would like to poke folks about a desktop integration pain point we (and our friends at other DEs) face with apps run via wine. Here's the relevant ticket: https://bugs.winehq.org/show_bug.cgi?id=32699 Let me give some broader context on this. Modern DEs have a swath of UX going on that relies on being able to identify the application owning a particular window in the windowing system. "Identify the application" in practice means "map the window to a .desktop file installed in the menu file system". We want to do this because: a) Many a taskbar/dock-like UI tries to form a smooth life cycle across "launcher button", "startup notification" and "window button(s)". For each of those we need to work out the app they are associated with to be able to match them up. If it fails, ugly things like co-existing task buttons or missing startup throbbers and similar happen. b) UI these days tends to prefer to load assets such as app icons via the values defined in .desktop files instead of using the window icon, to ensure consistent results (apps may load icon assets differently from the shell, leading to asset quality issues, inconsistent theming, the icon changing between the lifecycle states mentioned in 'a', etc.) Plus the Wayland windowing system we're all migrating towards currently doesn'ㅅ have window icons. c) DEs do things like track app starts/quits for things like "Most used apps" launcher lists. To match a window to a .desktop file we rely on window and process metadata. On X11, there's the WM_CLASS window property: https://tronche.com/gui/x/icccm/sec-4.html#WM_CLASS Ideally, the second part of a X11 window's WM_CLASS matches the name of a .desktop file installed on the system. Failing that, it matches the value of StartupWMClass in that .desktop file (this is the key part I'll get back to soon). Here's some more documentation by our friends at Gnome on this standard scheme: https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased (Look for WM_CLASS / StartupWMClass.) In practice, we pull many other tricks to cope with broken meta data. We try to match things by names, we grab process command lines from /proc and check them against Exec keys in .desktop files in complicated ways, ... it's a multi-page heuristic, and other DEs (Unity via bamf, Gnome Shell via iirc ShellWindowTracker) have equivalent code. Something Gnome does that we don't do (because I think it's brittle) is to store the name of the .desktop file an app was launched with as a private window hint on the next window that comes up (which then doesn't work when the app was launched from CLI instead of shell UI). None of these things currently allow us to handle wine very well. As of now (perhaps thanks to the patch mentioned in the linked ticket), wine sets WM_CLASS to the executable name of the app, e.g. "Notepad.exe". But the .desktop files it generates don't match this by name, and they don't contain a StartupWMClass key. The process command line is the Windows-like path to the .exe, which we also can't easily match up to the complicated Exec values in wine's .desktop files. (Aside: On Wayland, there's a more modern version of this: Windows have an appid property, which should be set to a .desktop file name. I don't know what the status of Wayland support in wine is -- currently the wine clients we encounter run over Xwayland, so we're back to WM_CLASS. We hope wine will set this property correctly when the time comes, or we're happy to work with you guys to find a way to support wine.) Long story short ... could this be picked up? The goal must be to have either .desktop file names or their StartupWMClass values match whatever is set as WM_CLASS. Currently, the easiest fix would be to set StartupWMClass to the executable name set as WM_CLASS. I assume the .desktop file generator has access to this, as the .lnk file referred to by Exec should presumably contain the name of the executable to be launched. Cheers, Eike

1 0

Re: [Tools] winetest: Get rid of $gitdir.
by Francois Gouget April 21, 2017

April 21, 2017

Actually this patch depends on the next one 'winetest/dissect: Check the build id and get its details in one Git call.' I'll resubmit them in a cleaner way. -- Francois Gouget <fgouget(a)codeweavers.com>

1 0

Re: [PATCH 1/4] regedit: Validate REG_SZ import data before processing it any further (v2)
by Alexandre Julliard April 21, 2017

April 21, 2017

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/regproc.c | 6 ++++++ > programs/regedit/tests/regedit.c | 14 +++++++------- > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/programs/regedit/regproc.c b/programs/regedit/regproc.c > index 96c4564..ccd9ebe 100644 > --- a/programs/regedit/regproc.c > +++ b/programs/regedit/regproc.c > @@ -293,6 +293,12 @@ static int REGPROC_unescape_string(WCHAR* str) > str[val_idx] = str[str_idx]; > break; > } > + } else if (str[str_idx] == '"') { > + WCHAR *p = str + str_idx + 1; > + while (*p == ' ' || *p == '\t') p++; > + if (*p && *p != ';') return 0; > + str[val_idx++] = str[str_idx]; > + break; That would work, but it seems to me that the comment check would be better in the caller, so that you can use the same code for validating value names. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] user32: add GetAutoRotationState/GetDisplayAutoRotationPreferences stubs
by Huw Davies April 21, 2017

April 21, 2017

On Thu, Apr 20, 2017 at 05:33:15PM -0500, Austin English wrote: > diff --git a/dlls/user32/misc.c b/dlls/user32/misc.c > index fa0502e..3581235 100644 > --- a/dlls/user32/misc.c > +++ b/dlls/user32/misc.c > @@ -910,3 +910,24 @@ LRESULT WINAPI ImeWndProcW( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) > > return DefWindowProcW(hwnd, msg, wParam, lParam); > } > + > +/********************************************************************** > + * GetAutoRotationState [USER32.@] > + */ > +BOOL WINAPI GetAutoRotationState( AR_STATE *state ) > +{ > + *state = AR_NOT_SUPPORTED; > + FIXME("(%p): stub\n", state); > + return TRUE; > +} > + I think these would be better in sysparams.c. I realise that there are already functions of this nature in misc.c - they could probably be moved across too. User32 does indeed have some odd 'misc' like functions, but these aren't them. Also, we'd generally put the FIXME() at the start of the function, not after the assignment. Apart from that, this looks good to me. Huw.

1 0

shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS
by 许文龙 April 20, 2017

April 20, 2017

shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS

2 1

Re: [05/10] webservices: Add support for outgoing TCP connections.
by Alexandre Julliard April 20, 2017

April 20, 2017

Hans Leidekker <hans(a)codeweavers.com> writes: > Signed-off-by: Hans Leidekker <hans(a)codeweavers.com> > --- > dlls/webservices/channel.c | 286 ++++++++++++++++++++++++++------- > dlls/webservices/listener.c | 16 +- > dlls/webservices/webservices_private.h | 5 + > 3 files changed, 240 insertions(+), 67 deletions(-) This breaks the Android build: x86_64-linux-android-gcc -m64 -c -o proxy.o ../../../wine/dlls/webservices/proxy.c -I. -I../../../wine/dlls/webservices \ -I../../include -I../../../wine/include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret In file included from ../../../wine/dlls/webservices/webservices_private.h:19:0, from ../../../wine/dlls/webservices/proxy.c:29: ../../../wine/include/ws2tcpip.h:57:16: error: conflicting types for 'socklen_t' ../../../wine/include/ws2tcpip.h:29:18: note: in definition of macro 'WS' In file included from /home/julliard/wine/build/toolchain-android-x86_64/sysroot/usr/include/stdio.h:50:0, from ../../../wine/dlls/webservices/proxy.c:19: /home/julliard/wine/build/toolchain-android-x86_64/sysroot/usr/include/sys/types.h:127:21: note: previous declaration of 'socklen_t' was here -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v3 PATCH 2/2] wined3d: Update only dirty regions in wined3d_device_update_texture().
by Marvin April 20, 2017

April 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=30284 Your paranoid android. === w864 (64 bit visual) === visual.c:7989: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). visual.c:8096: Test failed: Got unexpected color 0x00000080 for no color attribute test. === w1064 (64 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

1 0

Re: [v3 PATCH 1/2] wined3d: Add dirty region tracking.
by Marvin April 20, 2017

April 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=30282 Your paranoid android. === w864 (64 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff0000 for no color attribute test.

1 0

Re: [PATCH] mfplat: Implement MFCreateAttributes
by Nikolay Sivov April 20, 2017

April 20, 2017

On 04/20/2017 10:44 AM, Alistair Leslie-Hughes wrote: > +{ > + mfattributes *This = impl_from_IMFAttributes(iface); > + > + if(IsEqualGUID(riid, &IID_IUnknown) || > + IsEqualGUID(riid, &IID_IMFAttributes)) > + { > + TRACE("(IID_IUnknown %p)\n", object); > + *object = This; > + } > + else > + { > + FIXME("(%s, %p)\n", debugstr_guid(riid), object); > + *object = NULL; > + return E_NOINTERFACE; > + } > + > + IUnknown_AddRef((IUnknown*)*object); > + return S_OK; > +} Please add a regular TRACE for this method, and return iface or interface pointer from This, instead of This itself. Trace you have now is inaccurate. > +HRESULT WINAPI MFCreateAttributes(IMFAttributes **attributes, UINT32 size) > +{ > + mfattributes *object; > + > + TRACE("%p, %d\n", attributes, size); > + > + object = HeapAlloc( GetProcessHeap(), 0, sizeof(*attributes) ); > + if(!object) > + return E_OUTOFMEMORY; This should use sizeof(*object).

2 1

Re: [v2 PATCH] mfplat: Implement MFCreateAttributes
by Dmitry Timoshkov April 20, 2017

April 20, 2017

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +typedef struct _mfattributes > +{ > + IMFAttributes IMFAttributes_iface; > + LONG ref; > +} mfattributes; Please try to exchange these two lines in the structure above and see what happens. -- Dmitry.

1 0

Review | endproc.c: Implement ProcessPage_OnEndProcessTree
by Akarsha Sehwag April 19, 2017

April 19, 2017

Hi Could someone take out a little time to review the code once? Bug I intend to solve: https://bugs.winehq.org/show_bug.cgi?id=39640 I am new to wine. So, I would appreciate suggestions regarding my coding style if that's not-so-perfect according to wine standards. Or any other problems with the code I have written. I apologize for submitting an untested code earlier. I hope now it works fine. /*-------------------------------------------CODE--------------------------------------------------*/ typedef struct process_list { DWORD *pid; /*dynamic array to store the process IDs*/ SIZE_T count; /*index to maintain the last entry of the array;*/ SIZE_T size; /*the current size of the pid array*/ } process_list; static void init_process_list(process_list *list) { list->size = 4; /*initialise size with 4. Will increase if necessary.*/ list->pid = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, list->size * sizeof(list->pid)); list->count = 0; } static void increase_list_size(process_list *list) { list->size *= 2; list->pid = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, list->pid, list->size * sizeof(list->pid)); *//DOUBT: *Would sizeof(list->pid) be better or sizeof(int)? //Since in the former case, since list->pid is actually a pointer to DWORD, so //sizeof(list->pid) would return 8 bytes and not 4 bytes. } static void process_list_append(process_list *list, DWORD id) { if(list->count == list->size) increase_list_size(list); list->pid[list->count] = id; list->count += 1; } static void free_process_list(process_list *list) { HeapFree(GetProcessHeap(), 0, list->pid); } static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid) { PROCESSENTRY32 entry; SIZE_T start, end, i ; start = list->count; entry.dwSize = sizeof(entry); if(!Process32First(snapshot, &entry)) return; do { if(entry.th32ParentProcessID == pid) process_list_append(list, entry.th32ProcessID); } while (Process32Next(snapshot, &entry)); end = list->count; for(i = start; i < end; ++i) { enum_process_children(snapshot, list, list->pid[i]); } } void ProcessPage_OnEndProcessTree(void) { {...} process_list list; SIZE_T i; HANDLE snapshot; {...} init_process_list(&list); if(list.pid == NULL) return; process_list_append(&list, dwProcessId); snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); if(!snapshot) return; enum_process_children(snapshot, &list, dwProcessId); CloseHandle(snapshot); for(i = 0; i < list.count; ++i) { hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, list.pid[i]); if (!hProcess) { GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); break; } if (!TerminateProcess(hProcess, 0)) { GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); } CloseHandle(hProcess); } free_process_list(&list); } /*---------------------------------------END OF CODE------------------------------------------*/ Thanks and regards -- Akarsha Sehwag

1 0