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

Re: [PATCH 6/6] urlmon/tests: Added BindProtocol redirection handling tests.
by Alexandre Julliard June 6, 2017

June 6, 2017

Jacek Caban <jacek(a)codeweavers.com> writes: > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > dlls/urlmon/tests/protocol.c | 155 > ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 152 insertions(+), 3 deletions(-) This is failing consistently here: ../../../tools/runtest -q -P wine -T ../../.. -M urlmon.dll -p urlmon_test.exe.so protocol && touch protocol.ok protocol.c:1483: Test failed: unexpected call SetPriority protocol.c:1483: Test failed: unexpected call SetPriority Makefile:249: recipe for target 'protocol.ok' failed make: *** [protocol.ok] Error 2 -- Alexandre Julliard julliard(a)winehq.org

1 0

Wine uses the wrong pkg-config for 32 bit builds
by Francois Gouget June 6, 2017

June 6, 2017

The problem with pkg-config is that it has no idea what the build target is so it always returns the include and library paths appropriate for the build host. For instance when Wine tries to figure out how to use GStreamer 1.0 it calls: $ pkg-config --cflags gstreamer-1.0 -pthread -I/usr/include/gstreamer-1.0 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include It's pretty obvious that the 'x86_64-linux-gnu' is wrong for a 32 bit build. But then we never told pkg-config that we wanted to do a 32 bit build! There are two ways to tell pkg-config we want to do a 32 bit build. * The first way is to set PKG_CONFIG_LIBDIR to force pkg-config to only use the .pc files we want. However this requires knowing where the .pc files are stored and you'd want to make sure to include /usr/lib/<triplet>/pkgconfig, /usr/local/lib/<triplet>/pkgconfig. And then should you also include /usr/lib/pkgconfig? * Another way is to use a <triplet>-pkg-config tool, which will give us the right information for the specified <triplet>. Red Hat, Debian and its derivatives (Ubuntu, etc) all provide them. This gives: $ i686-linux-gnu-pkg-config --cflags gstreamer-1.0 -pthread -I/usr/include/gstreamer-1.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include We already have a WINE_CHECK_HOST_TOOL() macro that looks like it should do the trick. But it only takes ac_tool_prefix into account. So I modified it to take the host target into account if ac_tool_prefix is not set. This is modeled after the WINE_CHECK_MINGW_PROG macro and you can see the result in the patch below. On Red Hat the it's a quadruplet: it has an extra 'redhat-' in the name (it's i686-redhat-linux-gnu-pkg-config for instance). The patch below does not account for it. I'm hoping Red Hat developers can suggest how to best take this into account. Is this approach ok? ----- configure: Take the target host into account in WINE_CHECK_HOST_TOOL(). In particular this gets us the right pkg-config tool when building the 32 bit Wine. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- aclocal.m4 | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/aclocal.m4 b/aclocal.m4 index 9364188e334..4ac467a71e5 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -27,13 +27,17 @@ dnl dnl Like AC_CHECK_TOOL but without the broken fallback to non-prefixed name dnl AC_DEFUN([WINE_CHECK_HOST_TOOL], -[AS_VAR_SET_IF([ac_tool_prefix], - AC_CHECK_PROG([$1],[${ac_tool_prefix}$2],[${ac_tool_prefix}$2],,[$4])) -AS_VAR_IF([ac_cv_prog_$1],[], - [AS_VAR_IF([cross_compiling],[yes],[], - [AS_UNSET([ac_cv_prog_$1]) - AC_CHECK_PROG([$1],[$2],[$2],[$3],[$4])])], -[AS_VAR_COPY([$1],[ac_cv_prog_$1])])]) +[AS_VAR_IF([ac_tool_prefix],[], + [case "$host_cpu" in + i[[3456789]]86*) + ac_host_tool_list="m4_foreach([ac_wine_cpu],[i686,i586,i486,i386],[ac_wine_cpu-${host_os}-$2 ]) $2" ;; + x86_64) + ac_host_tool_list="x86_64-${host_os}-$2 $2" ;; + *) + ac_host_tool_list="" ;; + esac], + [ac_host_tool_tool_list="${ac_tool_prefix}-$2"]) + AC_CHECK_PROGS($1,$ac_host_tool_list,[$3],[$4])]) dnl **** Initialize the programs used by other checks **** dnl -- 2.11.0

4 11

Re: [PATCH v2 1/2] msvcrt: Clean up registered C++ object in handler.
by Piotr Caban June 6, 2017

June 6, 2017

Hi Daniel, On 06/06/17 00:43, Daniel Lehman wrote: > +typedef void (CALLBACK *__WINE_FINALLY_CTX)(BOOL, PEXCEPTION_RECORD, void*); The idea behind __FINALLY_CTX was to model native __finally behavior while extending its functionality for wine. It's not possible to call GetExceptionInfo/GetExceptionCode inside of finally on Windows so I don't think it's a change in good direction. If you really need the exception record you shouldn't use __FINALLY_CTX (but I don't think you need). > +static void CALLBACK cxx_catch_cleanup(BOOL normal, EXCEPTION_RECORD *rec, void *ctx) > +{ > + thread_data_t *data = msvcrt_get_thread_data(); > + cxx_frame_info *frame = ctx; > + > + if (normal) > + __CxxUnregisterExceptionObject(frame, FALSE); > + else > + { > + if (cxx_is_consolidate(rec)) > + rec = (void*)rec->ExceptionInformation[4]; > + > + __CxxUnregisterExceptionObject(frame, > + rec->ExceptionCode == CXX_EXCEPTION && > + data->exc_record->ExceptionCode == CXX_EXCEPTION && > + rec->ExceptionInformation[1] == data->exc_record->ExceptionInformation[1]); > + } Do you need to call __CxxUnregisterExceptionObject with TRUE as second argument in any case except of rethrow? > /* FIXME: native does local_unwind here in case of exception rethrow */ If you only need to handle rethrow in a special way I think it should be done differently. Something like this should do the job: __TRY { __TRY { ret_addr = handler(0, frame); } __EXCEPT(rethrow_filter) { /* probably all of this should be done inside of the filter */ /* do some unwinding here or add a FIXME */ catch_cleanup_ctx.in_use = TRUE; rethrow; } __ENDTRY; } __FINALLY_CTX(cxx_catch_cleanup, &catch_cleanup_ctx); The above code is incorrect if exception object needs to be preserved not only in case of rethrow. What do you think about it? Thanks, Piotr

2 1

Re: [PATCH 6/6] urlmon/tests: Added BindProtocol redirection handling tests.
by Marvin June 6, 2017

June 6, 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=31439 Your paranoid android. === w864 (32 bit protocol) === protocol.c:875: Test failed: unexpected call ReportProgress_CONNECTING === w864 (64 bit protocol) === protocol.c:875: Test failed: unexpected call ReportProgress_CONNECTING

1 0

Re: [v2 PATCH 2/3] mfplat: Implement MFCreateMediaType
by Nikolay Sivov June 6, 2017

June 6, 2017

On 31.05.2017 4:09, Alistair Leslie-Hughes wrote: > +typedef struct _mfmedaitype > +{ > + IMFMediaType IMFMediaType_iface; > + LONG ref; > +} mfmedaitype; You got a typo in structure name.

4 4

Re: [v3 PATCH] oleaut32: Improve support for IDispatch in VarCat
by Jacek Caban June 6, 2017

June 6, 2017

Hi Alistair, The patch looks mostly good now. On 26.05.2017 07:13, Alistair Leslie-Hughes wrote: > + hres = VariantChangeTypeEx(&bstrvar_left,tmp,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > + if (hres == DISP_E_TYPEMISMATCH) > { It seems that might be the right thing to do, but one more test would make sure about that. Please add a test that will hit this case without VT_DISPATCH nor VT_NULL, since both of them are kind of special. As far as I can see, a VT_BYREF with NULL reference would be good for that. > - hres = VariantChangeTypeEx(&bstrvar_left,left,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > - if (hres != S_OK) { > - VariantClear(&bstrvar_left); > - VariantClear(&bstrvar_right); > - return hres; > - } > + V_VT(&bstrvar_left) = VT_BSTR; > + V_BSTR(&bstrvar_left) = SysAllocString(sz_empty); You could just set it to NULL. Thanks, Jacek

1 0

Re: [PATCH 3/3] inetcomm: Implemented IMimeBody::GetClassID()
by Jacek Caban June 6, 2017

June 6, 2017

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

1 0

Re: [PATCH 2/3] inetcomm: Release previously set data (Valgrind)
by Jacek Caban June 6, 2017

June 6, 2017

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

1 0

Re: [PATCH 5/8] d3dx9/tests: Factor out test_effect_preshader_compare_pbool_consts() function.
by Matteo Bruni June 5, 2017

June 5, 2017

2017-06-02 11:58 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + if (!const_updated_mask) > + { > + for (i = 0; i < ARRAY_SIZE(test_effect_preshader_bconsts); ++i) > + { > + ok_(__FILE__, line)(!bdata[i] == !test_effect_preshader_bconsts[i], > + "Pixel shader boolean constants do not match, expected %#x, got %#x, i %u.\n", > + test_effect_preshader_bconsts[i], bdata[i], i); > + } > + } > + else > + { > + for (i = 0; i < ARRAY_SIZE(test_effect_preshader_bconsts); ++i) > + { > + if (const_updated_mask[i / TEST_EFFECT_BITMASK_BLOCK_SIZE] > + & (1u << (i % TEST_EFFECT_BITMASK_BLOCK_SIZE))) > + { > + ok_(__FILE__, line)(!bdata[i] == !test_effect_preshader_bconsts[i], > + "Pixel shader boolean constants do not match, expected %#x, got %#x, i %u, parameter %s.\n", > + test_effect_preshader_bconsts[i], bdata[i], i, updated_param); > + } > + else > + { > + ok_(__FILE__, line)(!bdata[i], > + "Pixel shader boolean constants updated unexpectedly, parameter %s.\n", updated_param); > + } > + } > + } Minor simplification possibility: in the (!const_updated_mask) case you could use a default "all ones" mask and use what's now the 'else' branch, getting rid of the 'if' branch altogether.

2 2

Re: [PATCH 5/5] d3drm: Consolidate GetClassName() implementation, reduce tests duplication
by Henri Verbeet June 5, 2017

June 5, 2017

On 5 June 2017 at 01:07, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +HRESULT d3drm_object_get_class_name(struct d3drm_object *object, DWORD *size, char *name) > +{ > + DWORD req_size; > + > + if (!size) > + return E_INVALIDARG; > + > + req_size = strlen(object->classname) + 1; > + if (name && *size < req_size) > + return E_INVALIDARG; > + > + *size = req_size; > + > + if (name) > + strcpy(name, object->classname); Note that you already know the string length, so just use memcpy(). (I.e., there's almost never a good reason to use strcpy().) > diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c > index e1091be423..a0dfe6c7d2 100644 > --- a/dlls/d3drm/tests/d3drm.c > +++ b/dlls/d3drm/tests/d3drm.c > @@ -52,6 +52,42 @@ static D3DRMMATRIX4D identity = { > { 0.0f, 0.0f, 0.0f, 1.0f } > }; > > + > +static void test_class_name(IDirect3DRMObject *object, const char *name) > +{ > + char cname[64] = {0}; > + HRESULT hr; > + DWORD size; > + > + hr = IDirect3DRMObject_GetClassName(object, NULL, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); "Got unexpected hr %#x.\n" > + hr = IDirect3DRMViewport_GetClassName(object, NULL, NULL); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + > + size = 0; > + hr = IDirect3DRMObject_GetClassName(object, &size, NULL); > + ok(hr == D3DRM_OK, "Cannot get classname size (hr = %x)\n", hr); > + ok(size == strlen(name) + 1, "wrong size: %u\n", size); > + > + size = 1; > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + ok(size == 1, "Got size %u.\n", size); > + > + size = sizeof(cname); > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == D3DRM_OK, "Cannot get classname (hr = %x)\n", hr); > + ok(size == strlen(name) + 1, "wrong size: %u\n", size); > + ok(!strcmp(cname, name), "Expected cname to be \"%s\", but got \"%s\"\n", name, cname); > + > + size = strlen(name); > + strcpy(cname, "XXX"); > + hr = IDirect3DRMObject_GetClassName(object, &size, cname); > + ok(hr == E_INVALIDARG, "GetClassName failed with %x\n", hr); > + ok(size == strlen(name), "wrong size: %u\n", size); > + ok(!strcmp(cname, "XXX"), "Expected unchanged buffer, but got \"%s\"\n", cname); > +} I would be happier if this printed the line number of the caller on failure, see e.g. check_interface_() in the d3d11 tests.

1 0

Re: [PATCH 1/5] d3drm/tests: Some tests for creating texture from ddraw surface
by Henri Verbeet June 5, 2017

June 5, 2017

On 5 June 2017 at 01:07, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d3drm/tests/d3drm.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 188 insertions(+) > It looks like a good part of this was inspired by the existing d3drm tests, and it doesn't appear to be any worse than those. However, we'd like to raise the standards a little. I.e., in the future, please use the d3d11 and ddraw tests as a reference, instead of the existing d3drm tests.

1 0

Re: [PATCH] shell32: Implement FCIDM_SHVIEW_CREATELINK
by Nikolay Sivov June 5, 2017

June 5, 2017

On 02.06.2017 17:24, Aric Stewart wrote: > > Presently the wine file explorer has a create shortcut entry that > does nothing. This implements the FCIDM_SHVIEW_CREATELINK command. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/shell32/shlview_cmenu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > + if (!SUCCEEDED(CoCreateInstance( &CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, > + &IID_IShellLinkW, (LPVOID*)&shelllink))) > + { > + ERR("couldn't create ShellLink object\n"); > + return; > + } This should be FAILED(). You can use IShellLink_Constructor() instead of going through generic object creation. > + SHGetSpecialFolderPathW(NULL, root, CSIDL_STARTMENU, TRUE); > + lnkfile=HeapAlloc(GetProcessHeap(), 0, (lstrlenW(root)+1+lstrlenW(wszFilename)+5) * sizeof(*lnkfile)); > + lstrcpyW(lnkfile, root); > + lstrcatW(lnkfile, backslashW); > + lstrcatW(lnkfile, wszFilename); > + lstrcatW(lnkfile, lnkW); Why is it created in Start Menu dir? I think usual behavior is to create it in target directory. > + if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile))) > + { > + ERR("couldn't get IPersistFile interface\n"); > + IShellLinkW_Release(shelllink); > + HeapFree(GetProcessHeap(), 0, lnkfile); > + return; > + } FAILED() again, but really it can't fail. Also please LPVOID -> void*.

2 2

Re: [PATCH] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Nikolay Sivov June 5, 2017

June 5, 2017

This needs more tests, for different msxml object versions, and for different input document. It's possible that get_xml() returns original document, so test loading non trivial document is necessary. I remember looking at this some time ago, not for this specific report but in general, and my conclusion was that the only way to generate matching document is to duplicate node dumping in our code.

1 0

Re: kernel32: Fix WriteConsoleOutputCharacter[AW] spec file entries
by Alexandre Julliard June 2, 2017

June 2, 2017

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -1612,8 +1612,8 @@ > @ stdcall WriteConsoleInputW(long ptr long ptr) > @ stdcall WriteConsoleOutputA(long ptr long long ptr) > @ stdcall WriteConsoleOutputAttribute(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterA(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterW(long ptr long long ptr) > +@ stdcall WriteConsoleOutputCharacterA(long str long long ptr) > +@ stdcall WriteConsoleOutputCharacterW(long wstr long long ptr) It's questionable to use string types on potentially non null-terminated strings. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/8] comcat: label ordinals
by Alexandre Julliard June 2, 2017

June 2, 2017

Austin English <austinenglish(a)gmail.com> writes: > Signed-off-by: Austin English <austinenglish(a)gmail.com> > --- > dlls/comcat/comcat.spec | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/dlls/comcat/comcat.spec b/dlls/comcat/comcat.spec > index 5b5f7b06cb..781b07437c 100644 > --- a/dlls/comcat/comcat.spec > +++ b/dlls/comcat/comcat.spec > @@ -1,4 +1,4 @@ > -@ stdcall -private DllCanUnloadNow() > -@ stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > -@ stdcall -private DllRegisterServer() > -@ stdcall -private DllUnregisterServer() > +1 stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > +2 stdcall -private DllCanUnloadNow() > +3 stdcall -private DllRegisterServer() > +4 stdcall -private DllUnregisterServer() It doesn't make much sense to define ordinals for such functions. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] user32: Keep tracking if we just created the menu.
by Huw Davies June 2, 2017

June 2, 2017

On Wed, May 31, 2017 at 02:34:06PM -0500, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/user32/menu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c > index a65f01eaa5..c52b723f76 100644 > --- a/dlls/user32/menu.c > +++ b/dlls/user32/menu.c > @@ -2597,7 +2597,10 @@ static BOOL MENU_ButtonDown( MTRACKER* pmt, HMENU hPtMenu, UINT wFlags ) > > /* If the popup menu is not already "popped" */ > if (!(ptmenu->items[pos].fState & MF_MOUSESELECT)) > + { > pmt->hCurrentMenu = MENU_ShowSubPopup( pmt->hOwnerWnd, hPtMenu, FALSE, wFlags ); > + return TRUE; > + } > } > > /* A click on an item or anywhere on a popup keeps tracking going */ So under what conditions does the if statement directly below this hunk not get triggered? Huw.

2 4

Re: [PATCH] fusion: use correct size for ZeroMemory (GCC7)
by Dmitry Timoshkov June 2, 2017

June 2, 2017

Marcus Meissner <meissner(a)suse.de> wrote: > diff --git a/dlls/fusion/tests/asmname.c b/dlls/fusion/tests/asmname.c > index 5fb14a4829..21cd4874ed 100644 > --- a/dlls/fusion/tests/asmname.c > +++ b/dlls/fusion/tests/asmname.c > @@ -358,7 +358,7 @@ static void test_assembly_name_props_line(IAssemblyName *name, > to_widechar(expect, vals[i].val); > > size = MAX_PATH; > - ZeroMemory(str, MAX_PATH); > + ZeroMemory(str, sizeof(str)); > hr = IAssemblyName_GetProperty(name, i, str, &size); > to_multibyte(val, str); Same problem with IAssemblyName_GetProperty() and 'size' argumenet as for the previous patch. -- Dmitry.

2 1

Re: [PATCH] use correct size for ZeroMemory (GCC7)
by Dmitry Timoshkov June 2, 2017

June 2, 2017

Marcus Meissner <meissner(a)suse.de> wrote: > --- a/dlls/fusion/tests/asmname.c > +++ b/dlls/fusion/tests/asmname.c > @@ -358,7 +358,7 @@ static void test_assembly_name_props_line(IAssemblyName *name, > to_widechar(expect, vals[i].val); > > size = MAX_PATH; > - ZeroMemory(str, MAX_PATH); > + ZeroMemory(str, sizeof(str)); > hr = IAssemblyName_GetProperty(name, i, str, &size); > to_multibyte(val, str); According to the implementation of IAssemblyName_GetProperty() 'size' assignment needs to be fixed as well. And IAssemblyName_GetProperty() implementation should be fixed to actually take 'size' into account. -- Dmitry.

1 0

Re: [PATCH 3/4] quartz: fix crash in FilterGraph2_Render
by Andrew Eikum June 2, 2017

June 2, 2017

Can you add a test for this? Andrew On Thu, May 25, 2017 at 04:54:49PM +0200, Miklós Máté wrote: > Note: there are a lot of other places as well where the null pointer handling > is present in the msdn docs but missing in wine. > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> > --- > dlls/quartz/filtergraph.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c > index a143b05d83..e26eb199c8 100644 > --- a/dlls/quartz/filtergraph.c > +++ b/dlls/quartz/filtergraph.c > @@ -1279,6 +1279,9 @@ static HRESULT WINAPI FilterGraph2_Render(IFilterGraph2 *iface, IPin *ppinOut) > > TRACE("(%p/%p)->(%p)\n", This, iface, ppinOut); > > + if (!ppinOut) > + return E_POINTER; > + > if (TRACE_ON(quartz)) > { > PIN_INFO PinInfo; > -- > 2.11.0 > > >

3 2

Re: [PATCH 1/4] quartz: AsyncReader is always expected to have output pin
by Andrew Eikum June 1, 2017

June 1, 2017

Thanks for working on this. I tested your patch sequence against a number of games as well as embedding videos in MSOffice 2007 and 2010 and found no regressions. I do have some comments, though, in-line below. Is this something you could write a test for? E.g. create the filter and then query for its output pin immediately after creation. On Thu, May 25, 2017 at 04:54:47PM +0200, Miklós Máté wrote: > Even between create() and Load(). This fixes a crash in DSPlayer. > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> > --- > dlls/quartz/filesource.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/dlls/quartz/filesource.c b/dlls/quartz/filesource.c > index ed7e5ee201..200463ff5b 100644 > --- a/dlls/quartz/filesource.c > +++ b/dlls/quartz/filesource.c > @@ -73,7 +73,8 @@ static const IFileSourceFilterVtbl FileSource_Vtbl; > static const IAsyncReaderVtbl FileAsyncReader_Vtbl; > static const IAMFilterMiscFlagsVtbl IAMFilterMiscFlags_Vtbl; > > -static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin); > +static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin); > +static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile); > > static const WCHAR mediatype_name[] = { > 'M', 'e', 'd', 'i', 'a', ' ', 'T', 'y', 'p', 'e', 0 }; > @@ -416,6 +417,7 @@ static const BaseFilterFuncTable BaseFuncTable = { > HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > { > AsyncReader *pAsyncRead; > + HRESULT hr; > > if( pUnkOuter ) > return CLASS_E_NOAGGREGATION; > @@ -429,7 +431,8 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > > pAsyncRead->IFileSourceFilter_iface.lpVtbl = &FileSource_Vtbl; > pAsyncRead->IAMFilterMiscFlags_iface.lpVtbl = &IAMFilterMiscFlags_Vtbl; > - pAsyncRead->pOutputPin = NULL; > + hr = FileAsyncReader_Construct(&pAsyncRead->filter.IBaseFilter_iface, &pAsyncRead->filter.csFilter, &pAsyncRead->pOutputPin); > + BaseFilterImpl_IncrementPinVersion(&pAsyncRead->filter); If this fails, I think we should free resources and return the error immediately. > > pAsyncRead->pszFileName = NULL; > pAsyncRead->pmt = NULL; > @@ -438,7 +441,7 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > > TRACE("-- created at %p\n", pAsyncRead); > > - return S_OK; > + return hr; > } > > /** IUnknown methods **/ > @@ -606,7 +609,7 @@ static ULONG WINAPI FileSource_Release(IFileSourceFilter * iface) > > static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFileName, const AM_MEDIA_TYPE * pmt) > { > - HRESULT hr; > + HRESULT hr = E_FAIL; > HANDLE hFile; > IAsyncReader * pReader = NULL; > AsyncReader *This = impl_from_IFileSourceFilter(iface); > @@ -625,16 +628,15 @@ static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFi > return HRESULT_FROM_WIN32(GetLastError()); > } > > - /* create pin */ > - hr = FileAsyncReader_Construct(hFile, &This->filter.IBaseFilter_iface, &This->filter.csFilter, &This->pOutputPin); > - BaseFilterImpl_IncrementPinVersion(&This->filter); > > - if (SUCCEEDED(hr)) > + if (This->pOutputPin) Since we now create the output pin during filter creation, this should never be non-NULL, right? > hr = IPin_QueryInterface(This->pOutputPin, &IID_IAsyncReader, (LPVOID *)&pReader); > > /* store file name & media type */ > if (SUCCEEDED(hr)) > { > + FileAsyncReader_SetFile(pReader, hFile); > + > CoTaskMemFree(This->pszFileName); > if (This->pmt) > FreeMediaType(This->pmt); > @@ -939,7 +941,7 @@ static const BaseOutputPinFuncTable output_BaseOutputFuncTable = { > BaseOutputPinImpl_BreakConnect > }; > > -static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin) > +static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin) > { > PIN_INFO piOutput; > HRESULT hr; > @@ -952,9 +954,8 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter > > if (SUCCEEDED(hr)) > { > - FileAsyncReader *pPinImpl = (FileAsyncReader *)*ppPin; > + FileAsyncReader *pPinImpl = impl_from_IPin(*ppPin); > pPinImpl->IAsyncReader_iface.lpVtbl = &FileAsyncReader_Vtbl; > - pPinImpl->hFile = hFile; > pPinImpl->bFlushing = FALSE; > pPinImpl->sample_list = NULL; > pPinImpl->handle_list = NULL; > @@ -965,6 +966,14 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter > return hr; > } > > +static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile) > +{ > + FileAsyncReader *This = impl_from_IAsyncReader(iface); > + > + This->hFile = hFile; > + return S_OK; > +} > + Why is this a separate function instead of just setting hFile in FileSource_Load? > /* IAsyncReader */ > > static HRESULT WINAPI FileAsyncReader_QueryInterface(IAsyncReader * iface, REFIID riid, LPVOID * ppv) > -- > 2.11.0 > > >

2 1

Re: ntdll: On macOS, don't use clock_gettime() even if it's available.
by Michael Müller June 1, 2017

June 1, 2017

Not sure which one of two commands is causing the lower overhead, but shouldn't you just build Wine using the 10.11 SDK if you plan to deploy it on 10.11? This could theoretically also apply to other parts of Wine and I don't think it makes sense to add backwards compatibility hacks everywhere. Regards, Michael

2 1

Re: [PATCH] msacm32: Correctly allocate WAVEFORMATEX.
by Andrew Eikum June 1, 2017

June 1, 2017

This doesn't look right to me. According to MSDN[1], the cbFormatSize field should already be the full size of the format struct. It's possible MSDN is wrong, but we'd need tests to prove that first. Who sets cbFormatSize? Could the error be there? [1] https://msdn.microsoft.com/en-us/library/windows/desktop/dd742918(v=vs.85).… Andrew On Wed, May 31, 2017 at 06:03:58PM -0500, Zebediah Figura wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=26388 > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/msacm32/format.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/msacm32/format.c b/dlls/msacm32/format.c > index de07cba9a3..ec46cae163 100644 > --- a/dlls/msacm32/format.c > +++ b/dlls/msacm32/format.c > @@ -75,7 +75,7 @@ static BOOL CALLBACK MSACM_FillFormatTagsCB(HACMDRIVERID hadid, > > afd.cbStruct = sizeof(afd); > afd.dwFormatTag = paftd->dwFormatTag; > - afd.pwfx = HeapAlloc(MSACM_hHeap, 0, paftd->cbFormatSize); > + afd.pwfx = HeapAlloc(MSACM_hHeap, 0, sizeof(WAVEFORMATEX) + paftd->cbFormatSize); > if (!afd.pwfx) return FALSE; > afd.pwfx->wFormatTag = paftd->dwFormatTag; > afd.pwfx->cbSize = paftd->cbFormatSize; > -- > 2.13.0 > > >

1 0

Re: PATCH 1/7] msvcrt: Translate SEH exception on each call to __CxxFrameHandler.
by Piotr Caban June 1, 2017

June 1, 2017

On 05/24/17 02:54, Daniel Lehman wrote: > +extern void __wine_rtl_unwind( EXCEPTION_REGISTRATION_RECORD* frame, EXCEPTION_RECORD *record, > + void (*target)(void) ); It's already defined in wine/exception.h. > +extern void CDECL __DestructExceptionObject(EXCEPTION_RECORD *rec); This is also already defined (in msvcrt.h) > + > +static void DECLSPEC_NORETURN seh_unwind_target(void) > +{ > + seh_frame_info *seh_frame = (seh_frame_info *)__wine_get_frame(); > + __wine_pop_frame( &seh_frame->frame ); There's no need to call __wine_pop_frame here, it will be done after the jump anyway. I think that the whole series will be cleaner/easier to read if we add the __EXCEPT_CTX/__FINALLY_CTX macros and use them here. I'm attaching a patch with proposition of changes for this functions. With such changes your patch will look like this (this is not tested): typedef struct { ULONG64 dest_frame; ULONG64 orig_frame; DISPATCHER_CONTEXT *dispatch; const cxx_function_descr *descr; } se_translator_ctx; static DWORD se_translation_filter(EXCEPTION_POINTERS *ep, void *c) { se_translator_ctx *ctx = (se_translator_ctx*)c; /* this is copied from original patch but I think that */ /* terminate should be called on non C++ exceptions here */ if (ep->rec->ExceptionCode != CXX_EXCEPTION) return ExceptionContinueSearch; find_catch_block(...); __DestructExceptionObject(ep->rec); return ExceptionExecuteHandler; } in cxx_frame_handler: if (data->se_translator) { se_translator_ctx ctx; //initialize ctx __TRY { except_ptrs.ExceptionRecord = rec; except_ptrs.ContextRecord = context; data->se_translator(rec->ExceptionCode, &except_ptrs); } __EXCEPT_CTX(se_translation_filter, &ctx) { } __END_TRY } With such changes second patch can be also changed to use __TRY/__FINALLY_CTX around "ret_addr = handler(0, frame);" file. What do you think about it? Thanks, Piotr

2 1